On Tue, Jan 28, 2025 at 7:03 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > On Tue, Jan 28, 2025 at 4:31 PM Junwang Zhao <zhjw...@gmail.com> wrote: > > > > On Tue, Jan 28, 2025 at 6:17 PM Ashutosh Bapat > > <ashutosh.bapat....@gmail.com> wrote: > > > > > > On Fri, Jan 24, 2025 at 9:16 PM Junwang Zhao <zhjw...@gmail.com> wrote: > > > > > > > > I figured out it's because I have `-DWRITE_READ_PARSE_PLAN_TREES` this > > > > option, > > > > removing this option clears the warning, but I'm not sure if this is a > > > > hidden issue. > > > > > > > > > > Thanks for the hint. I found the issue and the fix appears in 0004. > > > Some members of RangeTblEntry were not being handled in read/out > > > functions. > > > > Thanks for the fix, I will apply and test again. > > > > > > > > > > > > > > > 0001-0009 patches are the same as the previous set sans mergeconflict > > > > > fixes. > > > > > 0010 - is test for \dGx - to make sure that the extended format output > > > > > works with \dG. We may decide not to have this patch in the final > > > > > commit. But no harm in being there til that point. > > > > > > > > I have some changes based on the latest patch set. I attached two > > > > patches with > > > > the following summary. > > > > > > > > 0001: > > > > - doc: some of the query in ddl.sgml can not be executed > > > > > > The standard seems to require a property reference to be qualified by > > > element pattern variable, even if the property is referenced within > > > the same element pattern bound to the variable. Hence I accepted your > > > document fixes which qualify property reference with element pattern > > > variable. > > > > > > However, I didn't understand why you changed a LABEL name from order to > > > order_. > > > > Ah, that's because `order` is a keyword of SQL(order by), so the alias > > is a conflict. > > I'm not saying `order_` is a good name though. > > I didn't realize that. Thanks for catching. In that case just orders > or cust_orders?
Yeah, I think explicitly LABELed as `orders` makes sense. > > > > > > > > > > - after path factor was introduced, some comments doesn't apply > > > > > > Thanks pointing those out. Accepted after rephrasing those and also > > > correcting some related comments. > > > > > > > > > > > 0002: > > > > - add a get_propgraph_element_alias_name function and do some trivial > > > > refactor > > > > > > > > > > I see you have added some negative tests - object not found tests, but > > > I didn't see positive tests. Hence I haven't added those changes in > > > the attached patchset. But we certainly need those changes. You may > > > want to submit a patch with positive tests. That code needs to be > > > fixed before committing anyway. > > > > Ok, I'll add positive tests. > > Thanks. > > -- > Best Wishes, > Ashutosh Bapat -- Regards Junwang Zhao
