Thread: Ticket 119: handling opclass in dlgIndex
Hi, This patch adds a combobox in dlgIndex to allow a user to change the operator class for each column. I added a new column in the columns's list to show the non default opclass. Comments? BTW, I was wondering why the index's type is not set to btree by default. It is set to "", which really means btree. But the opclass code and the ASC/DESC and NULL FIRST/LAST code treat it as non btree, which is weird. I wonder if we can delete the blank type option. Another question. There is right now no way to change an index. We can alter its name, tablespace, stuff like that. But we can't really change its definition. I wonder if there would be a way to allow someone to change it with first dropping the old object and creating the new one. That would be better, for the user, than to make him drop the index and then recreate it completely. Not sure that my explanation is really clear :) -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Attachment
On Wed, Dec 23, 2009 at 3:45 PM, Guillaume Lelarge
<guillaume@lelarge.info> wrote:
> Hi,
>
> This patch adds a combobox in dlgIndex to allow a user to change the
> operator class for each column. I added a new column in the columns's
> list to show the non default opclass.
>
> Comments?
Seems to be some (long standing) inconsistency in our capitalisation
of multi-word labels - eg.
lstColumns->AddColumn(_("Column name"), 90);
lstColumns->AddColumn(_("Order"), 40);
lstColumns->AddColumn(_("NULLs Order"), 50);
+ lstColumns->AddColumn(_("Op. Class"), 40);
"Column name" vs. NULLs Order" for example.
I suspect you should remove this too:
+ wxLogError(wxT("opclass:") + GetOperatorClasses());
> BTW, I was wondering why the index's type is not set to btree by
> default. It is set to "", which really means btree. But the opclass code
> and the ASC/DESC and NULL FIRST/LAST code treat it as non btree, which
> is weird. I wonder if we can delete the blank type option.
Don't see why not.
> Another question. There is right now no way to change an index. We can
> alter its name, tablespace, stuff like that. But we can't really change
> its definition. I wonder if there would be a way to allow someone to
> change it with first dropping the old object and creating the new one.
> That would be better, for the user, than to make him drop the index and
> then recreate it completely. Not sure that my explanation is really clear :)
We'd really need an implementation of CREATE OR REPLACE INDEX ...
CONCURRENTLY to do that well I think. Otherwise, we could
inadvertently cause users great pain if they don't realise a
drop/create will happen for certain changes.
--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
Le 23/12/2009 17:47, Dave Page a écrit :
> On Wed, Dec 23, 2009 at 3:45 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> Hi,
>>
>> This patch adds a combobox in dlgIndex to allow a user to change the
>> operator class for each column. I added a new column in the columns's
>> list to show the non default opclass.
>>
>> Comments?
>
> Seems to be some (long standing) inconsistency in our capitalisation
> of multi-word labels - eg.
>
> lstColumns->AddColumn(_("Column name"), 90);
> lstColumns->AddColumn(_("Order"), 40);
> lstColumns->AddColumn(_("NULLs Order"), 50);
> + lstColumns->AddColumn(_("Op. Class"), 40);
>
> "Column name" vs. NULLs Order" for example.
>
I don't really know what the english standard way of casing words is. I
know that, in French, you're supposed to put the first character of a
string in uppercase. In English, I suppose I need to put the first
character of each word in uppercase. So, I think we need to upper case
the n in "Column name" (ie, "Column Name"). Can you confirm?
> I suspect you should remove this too:
>
> + wxLogError(wxT("opclass:") + GetOperatorClasses());
>
Damn, I deleted one just before sending the patch. Obviously I forgot
this one.
>> BTW, I was wondering why the index's type is not set to btree by
>> default. It is set to "", which really means btree. But the opclass code
>> and the ASC/DESC and NULL FIRST/LAST code treat it as non btree, which
>> is weird. I wonder if we can delete the blank type option.
>
> Don't see why not.
>
Will do. Should it be in another patch?
>> Another question. There is right now no way to change an index. We can
>> alter its name, tablespace, stuff like that. But we can't really change
>> its definition. I wonder if there would be a way to allow someone to
>> change it with first dropping the old object and creating the new one.
>> That would be better, for the user, than to make him drop the index and
>> then recreate it completely. Not sure that my explanation is really clear :)
>
> We'd really need an implementation of CREATE OR REPLACE INDEX ...
> CONCURRENTLY to do that well I think. Otherwise, we could
> inadvertently cause users great pain if they don't realise a
> drop/create will happen for certain changes.
>
Or we have to tell them we need to drop and create their index when they
click the OK button, perhaps giving them the choice to do it
concurrently. I wonder if there are other objects that would need that
treatment.
--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com
On Wed, Dec 23, 2009 at 6:08 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > I don't really know what the english standard way of casing words is. I > know that, in French, you're supposed to put the first character of a > string in uppercase. In English, I suppose I need to put the first > character of each word in uppercase. So, I think we need to upper case > the n in "Column name" (ie, "Column Name"). Can you confirm? Sorry - I meant to explain more but was multi-tasking and messed up :-( In grammatically correct English, only the first word of a sentence should be capitalised, but looking at other software like XCode and Firefox, it seems it's not that simple. In fact, as I'm trying to write this, I'm struggling to see a pattern in either of those apps - for example, in XCode's preferences pane I see options like: Automatically Suggest Suggestion delay Default File Encoding Tab indents So, I think we should use English rules, and only capitalise the first word, *except* for following words which have special meaning because they are SQL keywords for example, or acronyms. > Will do. Should it be in another patch? Probably better, if only to keep the commit logs focused on one change. > Or we have to tell them we need to drop and create their index when they > click the OK button, perhaps giving them the choice to do it > concurrently. I wonder if there are other objects that would need that > treatment. I can't say I'm that excited about that. It's probably safer for them to create the new index manually and then drop the old. I suppose we could create, drop, rename, but there's still something about the idea that gives me the heebie-jeebies. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Le 23/12/2009 20:04, Dave Page a écrit : > On Wed, Dec 23, 2009 at 6:08 PM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: > >> I don't really know what the english standard way of casing words is. I >> know that, in French, you're supposed to put the first character of a >> string in uppercase. In English, I suppose I need to put the first >> character of each word in uppercase. So, I think we need to upper case >> the n in "Column name" (ie, "Column Name"). Can you confirm? > > Sorry - I meant to explain more but was multi-tasking and messed up :-( > No problem :) > In grammatically correct English, only the first word of a sentence > should be capitalised, but looking at other software like XCode and > Firefox, it seems it's not that simple. In fact, as I'm trying to > write this, I'm struggling to see a pattern in either of those apps - > for example, in XCode's preferences pane I see options like: > > Automatically Suggest > Suggestion delay > Default File Encoding > Tab indents > > So, I think we should use English rules, and only capitalise the first > word, *except* for following words which have special meaning because > they are SQL keywords for example, or acronyms. > OK. I've done that on the dlgIndex source (.cpp and .xrc files). We need to do this on the complete source. New patch attached. >> Will do. Should it be in another patch? > > Probably better, if only to keep the commit logs focused on one change. > OK. I'll do in another patch, but I would like to commit this one before. >> Or we have to tell them we need to drop and create their index when they >> click the OK button, perhaps giving them the choice to do it >> concurrently. I wonder if there are other objects that would need that >> treatment. > > I can't say I'm that excited about that. It's probably safer for them > to create the new index manually and then drop the old. I suppose we > could create, drop, rename, but there's still something about the idea > that gives me the heebie-jeebies. > OK, not really a big issue anyways. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com