Обсуждение: Inconsistent ellipsis in regression test error message?
The most recent cfbot run for a patch I am interested in has failed a newly added regression test. Please see http://cfbot.cputube.org/ for 36/2906 ~ The failure logs [2] are very curious because the error message is what was expected but it has a different position of the ellipsis (...). But only for Windows. -- fail - publication WHERE clause must be boolean ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); ERROR: argument of PUBLICATION WHERE must be type boolean, not type integer -LINE 1: ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (... +LINE 1: ...PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); How is that possible? Is this a problem caused by the patch code? If so, how? Or is this some obscure boundary case bug of the error ellipsis calculation which I've exposed by accident due to the specific length of my bad command? Thanks for any ideas! ------ [2] https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.157408 Kind Regards, Peter Smith. Fujitsu Australia
On Fri, Dec 24, 2021 at 11:41:47AM +1100, Peter Smith wrote: > The most recent cfbot run for a patch I am interested in has failed a > newly added regression test. > > Please see http://cfbot.cputube.org/ for 36/2906 > > The failure logs [2] are very curious because the error message is > what was expected but it has a different position of the ellipsis > (...). > > But only for Windows. I reproduced the diff under linux with: time EXTRA_REGRESS_OPTS="--encoding=SQL_ASCII" make check # --no-locale The ellipsis is from reportErrorPosition(). I'm not sure I'll look into this more, though. > -- fail - publication WHERE clause must be boolean > ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); > ERROR: argument of PUBLICATION WHERE must be type boolean, not type integer > -LINE 1: ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (... > +LINE 1: ...PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); > Or is this some obscure boundary case bug of the error ellipsis > calculation which I've exposed by accident due to the specific length > of my bad command?
Peter Smith <smithpb2250@gmail.com> writes: > The most recent cfbot run for a patch I am interested in has failed a > newly added regression test. > Please see http://cfbot.cputube.org/ for 36/2906 > The failure logs [2] are very curious because the error message is > what was expected but it has a different position of the ellipsis That "expected" output is clearly completely insane; it's pointing the cursor in the middle of the "TABLE" keyword, not at the offending constant. I can reproduce that when the database encoding is UTF8, but if it's SQL_ASCII or a single-byte encoding then I get a saner result: regression=# ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); ERROR: argument of PUBLICATION WHERE must be type boolean, not type integer LINE 1: ...PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); ^ This is not a client-side problem: the error position being reported by the server is different, as you can easily see in the server's log: 2021-12-27 12:05:15.395 EST [1510837] ERROR: argument of PUBLICATION WHERE must be type boolean, not type integer at character33 2021-12-27 12:05:15.395 EST [1510837] STATEMENT: ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); (it says "at character 61" in the sane case). I traced this as far as finding that the pstate being passed to coerce_to_boolean has a totally wrong p_sourcetext: (gdb) p *pstate $3 = {parentParseState = 0x0, p_sourcetext = 0x1fba9e8 "{A_CONST :val 1234 :location 60}", p_rtable = 0x2063ce0, p_joinexprs = 0x0, p_joinlist = 0x0, p_namespace = 0x2063dc8, p_lateral_active = false, p_ctenamespace = 0x0, p_future_ctes = 0x0, p_parent_cte = 0x0, p_target_relation = 0x0, p_target_nsitem = 0x0, p_is_insert = false, p_windowdefs = 0x0, p_expr_kind = EXPR_KIND_NONE, p_next_resno = 1, p_multiassign_exprs = 0x0, p_locking_clause = 0x0, p_locked_from_parent = false, p_resolve_unknowns = true, p_queryEnv = 0x0, p_hasAggs = false, p_hasWindowFuncs = false, p_hasTargetSRFs = false, p_hasSubLinks = false, p_hasModifyingCTE = false, p_last_srf = 0x0, p_pre_columnref_hook = 0x0, p_post_columnref_hook = 0x0, p_paramref_hook = 0x0, p_coerce_param_hook = 0x0, p_ref_hook_state = 0x0} In short, GetTransformedWhereClause is inserting completely faulty data in p_sourcetext. This code needs to be revised to pass down the original command string, or maybe better pass down the whole ParseState that was available to AlterPublication, instead of inventing a bogus one. The reason why the behavior depends on DB encoding is left as an exercise for the student. regards, tom lane
On Tue, Dec 28, 2021 at 4:34 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Peter Smith <smithpb2250@gmail.com> writes: > > The most recent cfbot run for a patch I am interested in has failed a > > newly added regression test. > > Please see http://cfbot.cputube.org/ for 36/2906 > > The failure logs [2] are very curious because the error message is > > what was expected but it has a different position of the ellipsis > > That "expected" output is clearly completely insane; it's pointing > the cursor in the middle of the "TABLE" keyword, not at the offending > constant. I can reproduce that when the database encoding is UTF8, > but if it's SQL_ASCII or a single-byte encoding then I get a saner result: > > regression=# ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); > ERROR: argument of PUBLICATION WHERE must be type boolean, not type integer > LINE 1: ...PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); > ^ > > This is not a client-side problem: the error position being reported > by the server is different, as you can easily see in the server's log: > > 2021-12-27 12:05:15.395 EST [1510837] ERROR: argument of PUBLICATION WHERE must be type boolean, not type integer at character33 > 2021-12-27 12:05:15.395 EST [1510837] STATEMENT: ALTER PUBLICATION testpub5 SET TABLE testpub_rf_tbl3 WHERE (1234); > > (it says "at character 61" in the sane case). > > I traced this as far as finding that the pstate being passed to > coerce_to_boolean has a totally wrong p_sourcetext: > > (gdb) p *pstate > $3 = {parentParseState = 0x0, > p_sourcetext = 0x1fba9e8 "{A_CONST :val 1234 :location 60}", > p_rtable = 0x2063ce0, p_joinexprs = 0x0, p_joinlist = 0x0, > p_namespace = 0x2063dc8, p_lateral_active = false, p_ctenamespace = 0x0, > p_future_ctes = 0x0, p_parent_cte = 0x0, p_target_relation = 0x0, > p_target_nsitem = 0x0, p_is_insert = false, p_windowdefs = 0x0, > p_expr_kind = EXPR_KIND_NONE, p_next_resno = 1, p_multiassign_exprs = 0x0, > p_locking_clause = 0x0, p_locked_from_parent = false, > p_resolve_unknowns = true, p_queryEnv = 0x0, p_hasAggs = false, > p_hasWindowFuncs = false, p_hasTargetSRFs = false, p_hasSubLinks = false, > p_hasModifyingCTE = false, p_last_srf = 0x0, p_pre_columnref_hook = 0x0, > p_post_columnref_hook = 0x0, p_paramref_hook = 0x0, > p_coerce_param_hook = 0x0, p_ref_hook_state = 0x0} > > In short, GetTransformedWhereClause is inserting completely faulty data in > p_sourcetext. This code needs to be revised to pass down the original > command string, or maybe better pass down the whole ParseState that was > available to AlterPublication, instead of inventing a bogus one. > > The reason why the behavior depends on DB encoding is left as an > exercise for the student. > Thanks for the information, and sorry for taking up your time tracing what ended up being our bug after all... ------ Kind Regards, Peter Smith. Fujitsu Australia