On Sat, Jan 16, 2010 at 05:39, Robert Haas <robertmh...@gmail.com> wrote: > First, thanks for the review. Detailed comments/questions below. > > On Fri, Jan 15, 2010 at 12:52 AM, Alex Hunsaker <bada...@gmail.com> wrote: >> On Sun, Jan 10, 2010 at 12:27, Robert Haas <robertmh...@gmail.com> wrote: >>> I am not very happy with ATPrepSetOptions(). I basically just >>> retained the logic from ATPrepSetDistinct(), but it doesn't really >>> make sense in this context. The idea that we want to support >>> attdistinct for system tables and index columns was based on a very >>> specific understanding of what that was going to do; for attoptions, >>> well, it might make sense for the options that we have now, but it >>> might not make sense for the next thing we want to add, and there's >>> not going to be any easy fix for that. Even as it stands, the >>> n_distinct_inherited option is supported for both table columns and >>> index columns, but it only actually does anything for table columns. >> >> I say just do it in AT(Prep|Exec)SetOptions. We could extend struct >> relopt_gen... but that seems overkill and hard to do without knowing >> what else might be in attoptions. IMHO at this point its ok not to >> worry about it util we have something we actually care about >> restricting. > > I'm sorry - do what in AT(Prep|Exec)SetOptions?
Hrm lemme re-quote and try to slim it down a bit: >>> ... The idea that we want to support >>> attdistinct for system tables and index columns was based on a very >>> specific understanding of what that was going to do; for attoptions, >>> well, it might make sense for the options that we have now, but it >>> might not make sense for the next thing we want to add, and there's >>> not going to be any easy fix for that. Basically I was agreeing and saying when we add something new lets worry about it then. Clearer? >> ... The only >> perhaps interesting thing is when creating or adding an inherited >> table it does not pick up the parents attopts ... > > I don't think it should - it's fairly nonsensical for the current > options, at least. No argument here. :) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers