Re: Support for integrated tsearch configuration

Hi,

I have five days, full time, to work on this new feature. Here is my idea:

* add a "Text Search" node under the database node;
* add a Configurations node;
* add a Dictionarys node;
* add a Parsers node;
* add a Templates node.

Each of the four last nodes will have a property dialog.

I don't think there's much more things we can do to have a better
integration of the tsearch configuration.

Comments?

Regards.

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

On Fri, Jun 27, 2008 at 11:45 PM, Guillaume Lelarge
<guillaume(at)lelarge(dot)info> wrote:
> Hi,
>
> I have five days, full time, to work on this new feature. Here is my idea:

Thanks to Dalibo :-)

> * add a "Text Search" node under the database node;
> * add a Configurations node;
> * add a Dictionarys node;
> * add a Parsers node;
> * add a Templates node.
>
> Each of the four last nodes will have a property dialog.
>
> I don't think there's much more things we can do to have a better
> integration of the tsearch configuration.
>
> Comments?

Sounds pretty straightforward - except there may be some odd side
effects of the 'Text Search node. Everything is pretty much designed
around the idea that we alternate between collection nodes and object
nodes. There are some exceptions - just don't assume it'll be entirely
simple!

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Dave Page a écrit :
> On Fri, Jun 27, 2008 at 11:45 PM, Guillaume Lelarge
> <guillaume(at)lelarge(dot)info> wrote:
>> Hi,
>>
>> I have five days, full time, to work on this new feature. Here is my idea:
>
> Thanks to Dalibo :-)
>

:)

>> * add a "Text Search" node under the database node;
>> * add a Configurations node;
>> * add a Dictionarys node;
>> * add a Parsers node;
>> * add a Templates node.
>>
>> Each of the four last nodes will have a property dialog.
>>
>> I don't think there's much more things we can do to have a better
>> integration of the tsearch configuration.
>>
>> Comments?
>
> Sounds pretty straightforward - except there may be some odd side
> effects of the 'Text Search node. Everything is pretty much designed
> around the idea that we alternate between collection nodes and object
> nodes. There are some exceptions - just don't assume it'll be entirely
> simple!
>

Nothing is really simple on pgAdmin's coding (says the guy who tries
hardly to compile it under Windows since last evening... grmbl... :) ).

I was sure I was following the "collection nodes and objects nodes" way
of doing. Here are some ascii artwork:

- Databases (3)
+ DB1
+ DB2
- DB3
+ Catalogs (2)
+ Casts (0)
+ Languages (0)
+ Schemas (1)
+ Replication (0)
- Text Search
+ Configurations (2)
+ Dictionaries (2)
+ Parsers (2)
+ Templates (2)

Seems you're right. The Text search node is neither a collection neither
an object one. Unfortunately, I don't have a better one.

I need to think a bit more about this.

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

On Sat, Jun 28, 2008 at 1:54 PM, Guillaume Lelarge
> Nothing is really simple on pgAdmin's coding (says the guy who tries hardly
> to compile it under Windows since last evening... grmbl... :) ).

That shouldn't be hard - what was the problem?

> I was sure I was following the "collection nodes and objects nodes" way of
> doing. Here are some ascii artwork:
>
> - Databases (3)
> + DB1
> + DB2
> - DB3
> + Catalogs (2)
> + Casts (0)
> + Languages (0)
> + Schemas (1)
> + Replication (0)
> - Text Search
> + Configurations (2)
> + Dictionaries (2)
> + Parsers (2)
> + Templates (2)
>
> Seems you're right. The Text search node is neither a collection neither an
> object one. Unfortunately, I don't have a better one.

Logically I think the design is correct - the only 'more correct' way
would be to put the objects directly under a database, but that would
be quite ugly I think.

What you propose should work - it just might need a little thought.
Maybe we need a 'pgGroupObject' node type. Something to ponder on in
spare minutes rather than your 5 days I suspect :-)

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

> On Sat, Jun 28, 2008 at 1:54 PM, Guillaume Lelarge
> > Nothing is really simple on pgAdmin's coding (says the guy who tries
> hardly
> > to compile it under Windows since last evening... grmbl... :) ).
>
> That shouldn't be hard - what was the problem?

You really think so? Getting pgAdmin to compile in Windows is very
difficult. I thought it was done on purpose to make people dislike
Windows.

Jon

Roberts, Jon a écrit :
>> On Sat, Jun 28, 2008 at 1:54 PM, Guillaume Lelarge
>>> Nothing is really simple on pgAdmin's coding (says the guy who tries
>> hardly
>>> to compile it under Windows since last evening... grmbl... :) ).
>> That shouldn't be hard - what was the problem?
>
>
> You really think so? Getting pgAdmin to compile in Windows is very
> difficult. I thought it was done on purpose to make people dislike
> Windows.
>

I won't go as far as Jon, but it sure isn't easy. At least, for me.

But anyways, it works now ;) And I see your issue with my patch. I'll
work on it quite soon.

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

On Sat, Jun 28, 2008 at 7:20 PM, Roberts, Jon <Jon(dot)Roberts(at)asurion(dot)com> wrote:
>
> You really think so? Getting pgAdmin to compile in Windows is very
> difficult. I thought it was done on purpose to make people dislike
> Windows.

It might be difficult for users, but it shouldn't be for developers -
in fact, the instructions are shorter than they are for Unix or Mac
(which can be *much* harder to get a universal build working):

http://svn.pgadmin.org/cgi-bin/viewcvs.cgi/branches/REL-1_8_0_PATCHES/pgadmin3/INSTALL?view=markup

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Dave Page a écrit :
> On Sat, Jun 28, 2008 at 1:54 PM, Guillaume Lelarge
> [...]
>> I was sure I was following the "collection nodes and objects nodes" way of
>> doing. Here are some ascii artwork:
>>
>> - Databases (3)
>> + DB1
>> + DB2
>> - DB3
>> + Catalogs (2)
>> + Casts (0)
>> + Languages (0)
>> + Schemas (1)
>> + Replication (0)
>> - Text Search
>> + Configurations (2)
>> + Dictionaries (2)
>> + Parsers (2)
>> + Templates (2)
>>
>> Seems you're right. The Text search node is neither a collection neither an
>> object one. Unfortunately, I don't have a better one.
>
> Logically I think the design is correct - the only 'more correct' way
> would be to put the objects directly under a database, but that would
> be quite ugly I think.
>
> What you propose should work - it just might need a little thought.
> Maybe we need a 'pgGroupObject' node type. Something to ponder on in
> spare minutes rather than your 5 days I suspect :-)
>

I worked three days on it. The fourth was more about testing it on
different platforms (GTK, Windows, Mac). Now, It's working. I don't
attach the patch because it's really big, but here is a URL to get it
compressed:

http://developer.pgadmin.org/~guillaume/fts.patch.bz2

I was wrong on the layout of the text search objects inside the
treeview. They all belong to a schema, so they are part of a schema
node. You can see some screenshots on my blog:

* the treeview:
http://blog.guillaume.lelarge.info/public/pgadmin_fts/fts5.png

* text search configuration dialog:
http://blog.guillaume.lelarge.info/public/pgadmin_fts/fts3.png
(not uptodate screenshot, it lacks the Tokens tab)
http://blog.guillaume.lelarge.info/public/pgadmin_fts/fts9.png

* text search dictionary dialog:
http://blog.guillaume.lelarge.info/public/pgadmin_fts/fts7.png
http://blog.guillaume.lelarge.info/public/pgadmin_fts/fts8.png
(not uptodate screenshot, the add and changes buttons merge in a
single one)

* text search parser dialog:
http://blog.guillaume.lelarge.info/public/pgadmin_fts/fts6.png

* text search template dialog:
http://blog.guillaume.lelarge.info/public/pgadmin_fts/fts4.png

They all are resizable, so the ones that use wxListCtrl widget suffer
the Mac issue.

Comments?

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

On Fri, Aug 1, 2008 at 11:05 AM, Guillaume Lelarge
<guillaume(at)lelarge(dot)info> wrote:
>
> I worked three days on it. The fourth was more about testing it on different
> platforms (GTK, Windows, Mac). Now, It's working. I don't attach the patch
> because it's really big, but here is a URL to get it compressed:

Cool :-). The usual list of random thoughts....

- Should we call objects 'FTS xxxx'? All the 'Text Search xxxx' labels
look a little long.

- There are some tokens to add to the ctlSQLBox list - at least
GETTOKEN, LEXTYPES, HEADLINE, INIT, LEXIZE

- There is a little inconsistency in the RE-SQL formatting - for a
template for example we have:

CREATE TEXT SEARCH TEMPLATE fred (
INIT = dsimple_init,
LEXIZE = dsimple_lexize);

and for a dictionary:

CREATE TEXT SEARCH DICTIONARY fred (
TEMPLATE = "simple"
);

Note the ); position.

- I got a crash when trying to create a config with no tokens.

0 pgAdmin3-Debug 0x00021b63
wxArrayString::GetCount() const + 9 (arrstr.h:144)
1 pgAdmin3-Debug 0x000fed06
dlgTextSearchConfiguration::GetSql() + 1634
(dlgTextSearchConfiguration.cpp:346)
2 pgAdmin3-Debug 0x000cbf7f
dlgProperty::OnOK(wxCommandEvent&) + 335 (dlgProperty.cpp:759)
...

- The Dictionaries textbox is oddly sized on the Tokens tab of the
Configuration.

- The dialogue boxes default to different sizes. They should all be
consistently sized.

- Don't forget to add new headers to precomp.h.

I only gave the code a cusory glance - you've got lot's of pgAdmin
experience now so I trust that it's all as clean as the bits I looked
at :-)

Overall, looks pretty good :-)

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Dave Page a écrit :
> On Fri, Aug 1, 2008 at 11:05 AM, Guillaume Lelarge
> <guillaume(at)lelarge(dot)info> wrote:
>> I worked three days on it. The fourth was more about testing it on different
>> platforms (GTK, Windows, Mac). Now, It's working. I don't attach the patch
>> because it's really big, but here is a URL to get it compressed:
>
> Cool :-). The usual list of random thoughts....
>
> - Should we call objects 'FTS xxxx'? All the 'Text Search xxxx' labels
> look a little long.
>

Fixed. I put FTS on all menu items.

> - There are some tokens to add to the ctlSQLBox list - at least
> GETTOKEN, LEXTYPES, HEADLINE, INIT, LEXIZE
>

I suppose this is for highlighting the Text Search keywords?

I have a few questions about this. There's a wxString that lists all
PL/pgsql specific keywords. Should I do the same with Text Search ones?

Why are the PL/pgsql specific keywords out of the pgadmin/db/keywords.c
and pgadmin/include/parser/keywords.h files? And why are the
EnterpriseDB specific keywords in these files and not in a wxString like
the PL/pgsql ones?

Here are the FTS keywords I need to add:
* gettoken, lextypes, headline (FTS Parser)
* init, lexize (FTS Template)

These FTS keywords are already available:
* parser (FTS Configuration)
* template (FTS Dictionary)
* start, end (FTS Parser)

In the new patch, I'm using another wxString to make it work like the
PL/pgsql keywords.

> - There is a little inconsistency in the RE-SQL formatting - for a
> template for example we have:
>
> CREATE TEXT SEARCH TEMPLATE fred (
> INIT = dsimple_init,
> LEXIZE = dsimple_lexize);
>
> and for a dictionary:
>
> CREATE TEXT SEARCH DICTIONARY fred (
> TEMPLATE = "simple"
> );
>
> Note the ); position.
>

You're right. It's fixed to follow the second one.

> - I got a crash when trying to create a config with no tokens.
>
> 0 pgAdmin3-Debug 0x00021b63
> wxArrayString::GetCount() const + 9 (arrstr.h:144)
> 1 pgAdmin3-Debug 0x000fed06
> dlgTextSearchConfiguration::GetSql() + 1634
> (dlgTextSearchConfiguration.cpp:346)
> 2 pgAdmin3-Debug 0x000cbf7f
> dlgProperty::OnOK(wxCommandEvent&) + 335 (dlgProperty.cpp:759)
> ...
>

Fixed. I did so many tests that I didn't do this obvious one :/

> - The Dictionaries textbox is oddly sized on the Tokens tab of the
> Configuration.
>

Some explanations on the Dictionaries textbox. I wanted to have a real
textbox, so that anyone can type what they want in it (moreover, in the
order they want to have the dictionaries) and so that, if they choose
something in the combobox, it won't replace the actual content of the
textbox.

So, I shrinked the combobox to the lowest I can, so that users can't see
the text of the combobox, and I added a textbox before the combo box.
I'm not sure this is the best way to do it. I'm a bit afraid of some UI
issues but I didn't see one at the moment. And size seems good to me.

Can you send me a screenshot of your issue? Thanks.

> - The dialogue boxes default to different sizes. They should all be
> consistently sized.
>

Fixed. Unfortunately, now, the default size is also the minimum size.

> - Don't forget to add new headers to precomp.h.
>

Done.

> I only gave the code a cusory glance - you've got lot's of pgAdmin
> experience now so I trust that it's all as clean as the bits I looked
> at :-)
>
> Overall, looks pretty good :-)
>

Thanks :)

New patch on:
http://developer.pgadmin.org/~guillaume/fts_20080810.patch.bz2

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

On Sun, Aug 10, 2008 at 11:20 AM, Guillaume Lelarge
<guillaume(at)lelarge(dot)info> wrote:
>
> I suppose this is for highlighting the Text Search keywords?

Yes.

> I have a few questions about this. There's a wxString that lists all
> PL/pgsql specific keywords. Should I do the same with Text Search ones?

Yes please.

> Why are the PL/pgsql specific keywords out of the pgadmin/db/keywords.c and
> pgadmin/include/parser/keywords.h files? And why are the EnterpriseDB
> specific keywords in these files and not in a wxString like the PL/pgsql
> ones?

Because they are 'real' keywords as far as Postgres is concerned and
are used for quoting rules. FYI I've written a backend function for
8.4 which should simplify the maintenance of this list (well,
eliminate it in fact) - just need to add support to pgAdmin.

> Here are the FTS keywords I need to add:
> * gettoken, lextypes, headline (FTS Parser)
> * init, lexize (FTS Template)
>
> These FTS keywords are already available:
> * parser (FTS Configuration)
> * template (FTS Dictionary)
> * start, end (FTS Parser)
>
> In the new patch, I'm using another wxString to make it work like the
> PL/pgsql keywords.

Good.

>> - I got a crash when trying to create a config with no tokens.
>>
>> 0 pgAdmin3-Debug 0x00021b63
>> wxArrayString::GetCount() const + 9 (arrstr.h:144)
>> 1 pgAdmin3-Debug 0x000fed06
>> dlgTextSearchConfiguration::GetSql() + 1634
>> (dlgTextSearchConfiguration.cpp:346)
>> 2 pgAdmin3-Debug 0x000cbf7f
>> dlgProperty::OnOK(wxCommandEvent&) + 335 (dlgProperty.cpp:759)
>> ...
>>
>
> Fixed. I did so many tests that I didn't do this obvious one :/

:-) Looks good here.

>> - The Dictionaries textbox is oddly sized on the Tokens tab of the
>> Configuration.
>>
>
> Some explanations on the Dictionaries textbox. I wanted to have a real
> textbox, so that anyone can type what they want in it (moreover, in the
> order they want to have the dictionaries) and so that, if they choose
> something in the combobox, it won't replace the actual content of the
> textbox.

OK. BTW, if you use the combo when there's nothing in the textbox, you
get a leading ','

> So, I shrinked the combobox to the lowest I can, so that users can't see the
> text of the combobox, and I added a textbox before the combo box. I'm not
> sure this is the best way to do it. I'm a bit afraid of some UI issues but I
> didn't see one at the moment. And size seems good to me.
>
> Can you send me a screenshot of your issue? Thanks.

Attached. The top alignment of the button is different fromt he real
combo box above, and the right hand side of it is partially obscured
which makes it look a little odd.

>> - The dialogue boxes default to different sizes. They should all be
>> consistently sized.
>>
>
> Fixed. Unfortunately, now, the default size is also the minimum size.

That's how they usually work.

/D

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Dave Page a écrit :
> On Sun, Aug 10, 2008 at 11:20 AM, Guillaume Lelarge
> <guillaume(at)lelarge(dot)info> wrote:
> [...]
>> Why are the PL/pgsql specific keywords out of the pgadmin/db/keywords.c and
>> pgadmin/include/parser/keywords.h files? And why are the EnterpriseDB
>> specific keywords in these files and not in a wxString like the PL/pgsql
>> ones?
>
> Because they are 'real' keywords as far as Postgres is concerned and
> are used for quoting rules. FYI I've written a backend function for
> 8.4 which should simplify the maintenance of this list (well,
> eliminate it in fact) - just need to add support to pgAdmin.
>

Yep, I've seen this on pgsql-hackers list. It was on the july commit
fest (pg_get_keywords() function).

> [...]
>>> - The Dictionaries textbox is oddly sized on the Tokens tab of the
>>> Configuration.
>>>
>> Some explanations on the Dictionaries textbox. I wanted to have a real
>> textbox, so that anyone can type what they want in it (moreover, in the
>> order they want to have the dictionaries) and so that, if they choose
>> something in the combobox, it won't replace the actual content of the
>> textbox.
>
> OK. BTW, if you use the combo when there's nothing in the textbox, you
> get a leading ','
>

Fixed.

>> So, I shrinked the combobox to the lowest I can, so that users can't see the
>> text of the combobox, and I added a textbox before the combo box. I'm not
>> sure this is the best way to do it. I'm a bit afraid of some UI issues but I
>> didn't see one at the moment. And size seems good to me.
>>
>> Can you send me a screenshot of your issue? Thanks.
>
> Attached. The top alignment of the button is different fromt he real
> combo box above, and the right hand side of it is partially obscured
> which makes it look a little odd.
>

OK, I see what you mean. I had to replace the ctlComboBox with a
wxChoice component which integrates better.

New patch on:
http://developer.pgadmin.org/~guillaume/fts_20080811.patch.bz2

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com

On Mon, Aug 11, 2008 at 8:49 PM, Guillaume Lelarge
<guillaume(at)lelarge(dot)info> wrote:
> OK, I see what you mean. I had to replace the ctlComboBox with a wxChoice
> component which integrates better.
>
> New patch on:
> http://developer.pgadmin.org/~guillaume/fts_20080811.patch.bz2

Please apply - I'm not going to get time to look again for a while,
and I'm sure it's all good now :-)

--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com

Dave Page a écrit :
> On Mon, Aug 11, 2008 at 8:49 PM, Guillaume Lelarge
> <guillaume(at)lelarge(dot)info> wrote:
>> OK, I see what you mean. I had to replace the ctlComboBox with a wxChoice
>> component which integrates better.
>>
>> New patch on:
>> http://developer.pgadmin.org/~guillaume/fts_20080811.patch.bz2
>
> Please apply - I'm not going to get time to look again for a while,
> and I'm sure it's all good now :-)
>

Thanks, done.

If you later find any issues, I'll be glad to fix them.

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com