Greg:
Thanks for more debugging.
Cheers
On Thu, Feb 11, 2021 at 9:43 PM Greg Nancarrow <gregn4...@gmail.com> wrote:
> On Fri, Feb 12, 2021 at 3:21 PM Zhihong Yu <z...@yugabyte.com> wrote:
> >
> > Greg:
> > bq. we should just return parsetree->hasModifyingCTE at this point,
> >
> > Maybe you can clarify a bit.
> > The if (parsetree->hasModifyingCTE) check is followed by if
> (!hasModifyingCTE).
> > When parsetree->hasModifyingCTE is false, !hasModifyingCTE would be
> true, resulting in the execution of the if (!hasModifyingCTE) block.
> >
> > In your reply, did you mean that the if (!hasModifyingCTE) block is no
> longer needed ? (I guess not)
> >
>
> Sorry for not making it clear. What I meant was that instead of:
>
> if (parsetree->querySource == QSRC_ORIGINAL)
> {
> /* Assume original queries have hasModifyingCTE set correctly */
> if (parsetree->hasModifyingCTE)
> hasModifyingCTE = true;
> }
>
> I thought I should be able to use the following (it the setting for
> QSRC_ORIGINAL can really be trusted):
>
> if (parsetree->querySource == QSRC_ORIGINAL)
> {
> /* Assume original queries have hasModifyingCTE set correctly */
> return parsetree->hasModifyingCTE;
> }
>
> (and then the "if (!hasModifyingCTE)" test on the code following
> immediately below it can be removed)
>
>
> BUT - after testing that change, the problem test case (in the "with"
> tests) STILL fails.
> I then checked if hasModifyingCTE is always false in the QSRC_ORIGINAL
> case (by adding an Assert), and it always is false.
> So actually, there is no point in having the "if
> (parsetree->querySource == QSRC_ORIGINAL)" code block - even the so
> called "original" query doesn't maintain the setting correctly (even
> though the actual original query sent into the query rewriter does).
> And also then the "if (!hasModifyingCTE)" test on the code following
> immediately below it can be removed.
>
> Regards,
> Greg Nancarrow
> Fujitsu Australia
>
