Thread

  1. Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2026-05-12T06:20:55Z

    On Mon, May 4, 2026 at 8:42 PM Peter Eisentraut <peter@eisentraut.org> wrote:
    >
    > On 29.04.26 16:09, Ashutosh Bapat wrote:
    > > On Mon, Apr 27, 2026 at 11:34 AM SATYANARAYANA NARLAPURAM
    > > <satyanarlapuram@gmail.com> wrote:
    > >>
    > >> Hi,
    > >>
    > >> On Sun, Apr 26, 2026 at 8:10 AM Junwang Zhao <zhjwpku@gmail.com> wrote:
    > >>>
    > >>> Hi SATYANARAYANA,
    > >>>
    > >>> On Sun, Apr 26, 2026 at 3:53 AM SATYANARAYANA NARLAPURAM
    > >>> <satyanarlapuram@gmail.com> wrote:
    > >>>>
    > >>>> Hi hackers,
    > >>>>
    > >>>> transformRangeGraphTable() calls transformExpr() and
    > >>>> assign_list_collations() for COLUMNS expressions but missed calling
    > >>>> resolveTargetListUnknowns(). As a result, literals such as 'val1'
    > >>>> in a COLUMNS clause retained type "unknown", causing failures with
    > >>>> ORDER BY, UNION, and output conversions.
    > >>>>
    > >>>> Fix by calling resolveTargetListUnknowns() on the columns target
    > >>>> list right after assign_list_collations(), similar to SELECT target lists in
    > >>>> transformSelectStmt().
    > >>>>
    > >>>> Attached a patch to fix this, which also includes test cases to reproduce.
    > >>>
    > >>> I can reproduce this and the patch fixes it.
    > >>>
    > >>> One question: why is resolveTargetListUnknowns called after
    > >>> assign_list_collations?
    > >>>
    > >>> I'm asking because in transformSelectStmt, resolveTargetListUnknowns
    > >>> is invoked before assign_query_collations. It might not matter, but keeping
    > >>> the order consistent would be good for readers.
    > >>
    > >>
    > >> Updated the patch. It should be before.
    > >
    > > Do we really need a test for ORDER BY on a literal column? I replaced
    > > all the test queries with a single one which covers all the scenarios
    > > covered by those queries.
    > >
    > > The patch needed to consider pstate->p_resolve_unknowns. Unknown
    > > literals are not resolved as text always. See the test query.
    >
    > I couldn't find a commit to apply this patch cleanly (the subject says
    > patch 5/5, so maybe you had some unpublished local changes?).
    
    I am maintaining all the SQL/PGQ fixes in the same branch and creating
    patches from that branch. Hence 5/5. But still it shouldn't have
    applied cleanly. Both git cherry-pick and git am <attached patch
    file>, on a fresh branch succeeded. Please let me know if you still
    face the issue.
    
    > After
    > applying the test case manually, it looks like the test output is
    > already correct without the code change.  So if this patch is still
    > required, we need a better test case.
    >
    
    Yes, the test query doesn't reproduce the bug. Actually all but only
    two queries from the Satya's patch show the bug. I have included a
    test query which fails, with expected error message, without code
    changes now. Also I don't think we need a separate section for this
    test query. Included the query in one of the earliest sections in the
    file.
    
    -- 
    Best Wishes,
    Ashutosh Bapat