On Mon, Apr 24, 2023 at 8:18 AM shveta malik <shveta.ma...@gmail.com> wrote: > > On Thu, Apr 20, 2023 at 3:40 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Thu, Apr 20, 2023 at 9:11 AM shveta malik <shveta.ma...@gmail.com> wrote: > > > > > > > > > Few comments for ddl_deparse.c in patch dated April17: > > > > > > > > 6) There are plenty of places where we use 'append_not_present' > > > without using 'append_null_object'. > > > Do we need to have 'append_null_object' along with > > > 'append_not_present' at these places? > > > > > > > What is the difference if we add a null object or not before not_present? > > > > Sorry I missed this question earlier. There is not much difference > execution wise, The "present": false' attribute is sufficient to > indicate that the expansion of that element is not needed when we > convert back json to ddl command. It is only needed for 'verbose' > mode. The 'append_null_object' call makes the format string complete > for the user to understand it better. Example: > > --without append_null_object, we get: > "collation": {"fmt": "COLLATE", "present": false} > > --with append_null_object, we get: > With append_null_object(tmp_obj, "%{name}D"), it is: > "collation": {"fmt": "COLLATE %{name}D", "name": null, "present": false} > > So fmt: "COLLATE %{name}D" is more complete (even though not needed > when the COLLATE clause is absent) and thus aligns to what we expect > out of verbose mode. >
Thanks for the explanation. However, I feel we can split this verbose handling/optimization into a separate patch so that we can focus on the core part first. -- With Regards, Amit Kapila.