Thread: Inconsistent ellipsis in regression test error message?

Inconsistent ellipsis in regression test error message?

From
Peter Smith
Date:
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



Re: Inconsistent ellipsis in regression test error message?

From
Justin Pryzby
Date:
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?



Re: Inconsistent ellipsis in regression test error message?

From
Tom Lane
Date:
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



Re: Inconsistent ellipsis in regression test error message?

From
Peter Smith
Date:
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