RFC: Quotable names

classic Classic list List threaded Threaded
35 messages Options
12
Reply | Threaded
Open this post in threaded view
|

RFC: Quotable names

Milan Babuskov-2
Hi all,

I was looking at our "major feature" list, and starting to think how to
implement the "Quotable identifiers (not ignoring case)" painlessly.

Explanation for those of you with cheaper tickets: it is needed in SQL
scripts when object names are case sensitive (contain lower-case
characters) or object names are equal to some keyword. Example 1:

create table "Employee"
create table Employee

The first one creates table Employee, while the other table EMPLOYEE.

Example 2:

create table "Table"
create table Table

The first one creates table named Table, while the second one fails.


So, in order to work with those objects, we need to quote their names.
My idea is to do this as transparently as possible, without any
intervention needed bu the user. I propose we add a new function
MetadataItem::getQuotableName or getScriptableName, or something like
that (I'm open to suggestions), which would do something like this:

wxString MetadataItem::getQuotableName()
{
        temp = getName(); << this would call virtual getName

        if (isReserved(temp) || containsLowerCase(temp))
                return "\"" + temp + "\"";
        else
                return temp;
}

We would, of course change all the statement-generating code to use this
instead of getName(). We would still use unquoted name to show it in the
tree, property pages, etc.

It would be good that isReserved and containsLowerCase are publicly
available static functions (we could call them some other name, or even
clamp them together), so that we can all those even if we only got a
string somewhere (for example: adding a new field).

I would leave two options for the user, though:
1. quote everything (off by default)
2. ignore case (on by default)

By "ignore case" I mean that when user opens FieldPropertiesFrame, adds
a new field giving it a name with lowercase letters, FR would not quote
it. It could be called "ignore case of user supplied names" or something
like that.

BTW, Some other code would need to be changed too, like statement parser
for example (currently we don't support quoted identifiers at all).
Also, auto-complete might have to be modified, and who knows what else.
I'm going to work on all this soon, so if you have any work pending, let
me know.

P.S. I plan to work on and complete major features 2, 3, and 5 from the
list before the end of this year. After they're done, I'm going to work
on 4 too. So, I leave nr. 1 for you guys if you're interested... ;)

--
Milan Babuskov
http://fbexport.sourceforge.net
http://www.flamerobin.org



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Milan Babuskov-2
Milan Babuskov wrote:
> It would be good that isReserved and containsLowerCase are publicly
> available static functions (we could call them some other name, or even
> clamp them together), so that we can all those even if we only got a
> string somewhere (for example: adding a new field).

We also need isAllAscii() here, as it appears that non-ascii names need
to be quoted to. Perhaps we should make a single function named
needsQuoting() and put everything inside?

--
Milan Babuskov
http://fbexport.sourceforge.net
http://www.flamerobin.org



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Michael Hieke
In reply to this post by Milan Babuskov-2
Milan,

Milan Babuskov wrote:

> I was looking at our "major feature" list, and starting to think how to
> implement the "Quotable identifiers (not ignoring case)" painlessly.

I have thought about quoted identifiers when thinking about the syntax
scanner/parser that we need, and I believe we won't get them painlessly.
  More than that, I think we need to invest a little time to clean up FR
internals, before we implement any new features.

> So, in order to work with those objects, we need to quote their
> names. My idea is to do this as transparently as possible, without
> any intervention needed bu the user.

Agreed.

> I propose we add a new function MetadataItem::getQuotableName or
> getScriptableName, or something like that (I'm open to suggestions)

We should not decide whether a name needs to be quoted, but instead keep
the DBH object names, quoted or unquoted, as we got them - either from
the database, or from the user.  We should only ever *add* quotes when
necessary (for instance when a reserved word in upper case letters is
read as the column name from the database), but never remove quotes.

> We would, of course change all the statement-generating code to use
> this instead of getName(). We would still use unquoted name to show
> it in the tree, property pages, etc.

Again, we should show all names as they *are*.

> It would be good that isReserved and containsLowerCase are publicly
> available static functions (we could call them some other name, or even
> clamp them together), so that we can all those even if we only got a
> string somewhere (for example: adding a new field).
>
> I would leave two options for the user, though:
> 1. quote everything (off by default)
> 2. ignore case (on by default)
>
> By "ignore case" I mean that when user opens FieldPropertiesFrame, adds
> a new field giving it a name with lowercase letters, FR would not quote
> it. It could be called "ignore case of user supplied names" or something
> like that.

More user options are a bad idea. IMNSHO we are already bloating FR with
a lot of user options.  Adding more options for quoted identifiers isn't
going to fly, instead the code should be smart enough to decide for
itself whether quotes are needed.  For your example I would prefer a
checkbox in the FieldPropertiesDialog over a global option.


Regarding my first comment:

A few things should IMHO be fixed before we attempt to create new
features.  I could list many of those, but here are two that are closely
connected with the quoted identifiers feature:

To really support quoted identifiers transparently we need to be able to
compare DBH object names, regardless of where they come from.  Right now
FR uses "==" everywhere to compare names, which is wrong for unquoted
identifiers, and I believe there are any number of ways to trick the
statement parsing in FR, just by playing with char cases and quotes.

The first fundamental change needed is the replacement of current

   wxString nameM;

in MetadataItem with something like

   Identifier nameM;

a class which has methods to compare with another Identifier / wxString
/ std::string for equality.  This Identifier class would contain the
name in exactly the given char case, and it would maybe have a field
isQuotedM (maybe something like a tree-state: quoted, unquoted,
as-needed (?)).

The second important change is the introduction of a syntax
scanner/parser; without it support for quoted identifiers in
parseStatements() would mostly introduce write-only code (the sort that
can be written, but never again be read).

I know that this sounds kind of negative, and that this kind of
refactoring is both difficult and brings no user-visible progress.  But
I doubt that FR will stay maintainable when features are piled on top of
  a foundation that isn't sound.

For me it isn't really maintainable right now.
I have to say that several times during the last weeks I started to work
on some part of FR source code or another, only to give up after a
couple of hours.  Either because my changes had side-effects I did not
understand, or because necessary changes started to touch a lot of
files, or because I felt not safe changing the internals of
ExecuteSqlFrame.cpp (1956 lines at the time of writing...).

Do you really feel good with adding more code (== features) to FR,
without cleaning up the base first?

Thanks

--
Michael Hieke


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Gregory Sapunkov
В Чтв, 13/10/2005 в 23:11 +0200, Michael Hieke пишет:

Hello guys!

> Milan,
>
> Milan Babuskov wrote:
>
> > I was looking at our "major feature" list, and starting to think how to
> > implement the "Quotable identifiers (not ignoring case)" painlessly.
>
> I have thought about quoted identifiers when thinking about the syntax
> scanner/parser that we need, and I believe we won't get them painlessly.
>   More than that, I think we need to invest a little time to clean up FR
> internals, before we implement any new features.
>

+1

IMO "Double-quoted identifiers" is a more "widely" subject than just an
option of quoting of reserved words and low-case words.

BTW "...Firebird does permit a slight relaxation..." in using of
double-quoted identifiers without quotes in some cases:
http://firebird.sf.net/manual/qsg15-firebird-sql.html#qsg15-strings
(Double-quoted identifiers)

p.s.
You've done great job! my congratulations.

it was a hard year for me... and, what is more importantly for you - it
was annoying for you. I'm talking about my "unperformed liability". ok,
i say it before and i can say it again - "I'm sorry".

I want to return to flamerobin project. And I'm going to do it from now.

I've got several questions (may i?):

1. What versions of wxWidgets we are use? (2.6.2)

2. AFAIU the IBPP code is directly integrated into code-base. (to make
life more easy?)

3. About DBH (yeah...)
- wxString was preferred to avoid converting from std::string wxWidgets?
yes? I like the Michael idea about Identifier class, so I will not
discuss this.
- what about "smart pointers"? did you decide anything?

I'm not going to be a locomotive-programmer (-:
If you have nothing against it I'll return to DBH "reinventing".

Any suggestions?


--
Gregory Sapunkov



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Kjell Rilbe
In reply to this post by Michael Hieke
Hi all,

A couple of comments from my perspective as a user...

Michael Hieke wrote:

>> I propose we add a new function MetadataItem::getQuotableName or
>> getScriptableName, or something like that (I'm open to suggestions)
>
> We should not decide whether a name needs to be quoted, but instead keep
> the DBH object names, quoted or unquoted, as we got them - either from
> the database, or from the user.  We should only ever *add* quotes when
> necessary (for instance when a reserved word in upper case letters is
> read as the column name from the database), but never remove quotes.

But if the user is used to getting idents converted to uppercase it
would probably be confusing to have input "mytable" interpreted as
"\"mytable\"" instead of "MYTABLE". Apart from that, I agree with Michael.

>> We would, of course change all the statement-generating code to use
>> this instead of getName(). We would still use unquoted name to show
>> it in the tree, property pages, etc.
>
>
> Again, we should show all names as they *are*.

Of course. Showing an identifier in different case than it actually has
*anywhere* would be very confusing.

> More user options are a bad idea. IMNSHO we are already bloating FR with
> a lot of user options.  Adding more options for quoted identifiers isn't
> going to fly, instead the code should be smart enough to decide for
> itself whether quotes are needed.  For your example I would prefer a
> checkbox in the FieldPropertiesDialog over a global option.

I would *not* want FR to mix quoted and unquoted identifiers. If I
happen to have a table with columns "ID" and "Name" I would *not* want
the autogenerated select SQL to appear like this:

select ID, "Name"
from "MyTable"

Please quote "ID" too. I might be able to accept that FR automatically
chooses quotes or not on a per statement basis. So, autogenereated SQL
for a table whose name and column names are all uppercase plain ascii,
go ahead and drop quotes. But if one single identifier is quoted, then
quote them all.

Mixing quoted and unquoted seems extremely error prone to me.

Seems to me like a good idea to clean up FR code before adding features.
That's what I would do if it were my project and I didn't have a hard
deadline for the features. Even if I had a hard deadline, I'd try to
move it to be able to sqeeze in the cleanup first.

Kjell
--
--------------------------------------
Kjell Rilbe
Adressmarknaden AM AB
E-post: [hidden email]
Telefon: 08-761 06 55
Mobil: 0733-44 24 64


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Michael Hieke-2
Kjell,

Kjell Rilbe wrote:

> But if the user is used to getting idents converted to uppercase it
> would probably be confusing to have input "mytable" interpreted as
> "\"mytable\"" instead of "MYTABLE".

there might be a place for a global option here, indeed.

> I would *not* want FR to mix quoted and unquoted identifiers. If I
> happen to have a table with columns "ID" and "Name" I would *not*
> want the autogenerated select SQL to appear like this:
>
> select ID, "Name"
> from "MyTable"
>
> Please quote "ID" too. I might be able to accept that FR
> automatically chooses quotes or not on a per statement basis. So,
> autogenereated SQL for a table whose name and column names are all
> uppercase plain ascii, go ahead and drop quotes. But if one single
> identifier is quoted, then quote them all.
>
> Mixing quoted and unquoted seems extremely error prone to me.

Ok, mixing quoted and unquoted does not bother me personally, but I find
your POV equally valid.  And this automatic decision whether to use
quotes looks much better to me than a user option.

Thanks

--
Michael Hieke


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Kjell Rilbe
Michael,

Michael Hieke wrote:

> Kjell Rilbe wrote:
>
> Ok, mixing quoted and unquoted does not bother me personally, but I find
> your POV equally valid.  And this automatic decision whether to use
> quotes looks much better to me than a user option.

Thank you. I would prefer to have all identifiers quoted always - I've
gotten used to that. But the automatic decision on a per statement basis
is acceptable. (In other words - I'd like to have the global option, but
I see the problem with too many options so let's drop it.)

Kjell
--
--------------------------------------
Kjell Rilbe
Adressmarknaden AM AB
E-post: [hidden email]
Telefon: 08-761 06 55
Mobil: 0733-44 24 64


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Milan Babuskov-5
In reply to this post by Michael Hieke
Michael Hieke wrote:
> I have thought about quoted identifiers when thinking about the syntax
> scanner/parser that we need, and I believe we won't get them painlessly.
>  More than that, I think we need to invest a little time to clean up FR
> internals, before we implement any new features.

Cleaning up internals usually involves you or Nando, as I'm the one who
made the mess in the first place ;) But, I see that the pace is getting
slow. Anyway, adding new features can bring some other things that need
fixing on surface, and gives a better perspective.

>> I propose we add a new function MetadataItem::getQuotableName or
>> getScriptableName, or something like that (I'm open to suggestions)
>
> We should not decide whether a name needs to be quoted, but instead keep
> the DBH object names, quoted or unquoted, as we got them - either from
> the database, or from the user.  We should only ever *add* quotes when
> necessary (for instance when a reserved word in upper case letters is
> read as the column name from the database), but never remove quotes.

I disagree. I would hate to see a quoted name in main tree, property
page, etc. It would also make all the sorting look dumb, as all quoted
stuff would come first, and unquoted later. Besides, even if user
creates an object using SQL statement with quotes, FB will return it
unquoted back to us. Sounds like a potential for a big mess. IMO, we
should strip quotes always, and only add them when needed.

>> We would, of course change all the statement-generating code to use
>> this instead of getName(). We would still use unquoted name to show
>> it in the tree, property pages, etc.
>
> Again, we should show all names as they *are*.

Please, a table name Milan is Milan and not "Milan". The fact it needs
to be written as "Milan" in SQL scripts is just a feature of Firebird,
and object's name would still be stored as Milan in system tables.

> More user options are a bad idea. IMNSHO we are already bloating FR with
> a lot of user options.  Adding more options for quoted identifiers isn't
> going to fly, instead the code should be smart enough to decide for
> itself whether quotes are needed.

Ok.

> For your example I would prefer a
> checkbox in the FieldPropertiesDialog over a global option.

Good idea, agreed.

> The first fundamental change needed is the replacement of current
>
>   wxString nameM;
>
> in MetadataItem with something like
>
>   Identifier nameM;
>
> a class which has methods to compare with another Identifier / wxString
> / std::string for equality.

Good idea. I'll look into that direction before considering further
steps. So, Identifier class would then have "getName" and
"getQuotedName" methods? We wouldn't ask for MetadataItem::getName() but
rather

MetadataItem::getName()::getString()  and
MetadataItem::getName()::getQuotedString()  or something like that?

Before you comment on it again: The only place where we should see the
quotes are the SQL scripts! That's why we need those two functions.

> For me it isn't really maintainable right now.
> I have to say that several times during the last weeks I started to work
> on some part of FR source code or another, only to give up after a
> couple of hours.  Either because my changes had side-effects I did not
> understand

You should ask on the list, or me privately if not appropriate. I
wondered why the list was so quiet.

>, or because necessary changes started to touch a lot of
> files, or because I felt not safe changing the internals of
> ExecuteSqlFrame.cpp (1956 lines at the time of writing...).

I admit it is a behemoth, as I even wrote about it not so long ago.

> Do you really feel good with adding more code (== features) to FR,
> without cleaning up the base first?

Depends on level of their involvement with other code. I feel that I can
easily add user management/grants, without much trouble, and that's my
next step. Also, DDL script generator for all objects doesn't look too
hard (if you skip this Identifier issue we discussed today) - a visitor
for each object type and we're done (more on that in some other thread,
later). OTOH, I do feel that stuff like multithreaded query executing
for example, would be very hard to add before cleanup of ExecuteSqlFrame.

It's just that I need some features now, so I don't have to use
command-line tools, and I wish to implement them. I won't mind helping
the rewrite of that code to some new architecture.

M.


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Milan Babuskov-5
In reply to this post by Gregory Sapunkov
Gregory Sapunkov wrote:
> I want to return to flamerobin project. And I'm going to do it from now.

Glad to have you back, again. :)

> I've got several questions (may i?):
>
> 1. What versions of wxWidgets we are use? (2.6.2)

Any 2.6.x is fine.

> 2. AFAIU the IBPP code is directly integrated into code-base. (to make
> life more easy?)

Yes. Just checkout the sources from CVS.

> 3. About DBH (yeah...)
> - wxString was preferred to avoid converting from std::string wxWidgets?
> yes?

wxString is preferred since we need to do proper translations of
characters, and keeping it at database-layer-access-points reduces the
problems. We still don't have benefit from it, but we will.

> - what about "smart pointers"? did you decide anything?

Idea is good, but takes too much time and touches too much code. If you
ask me, I'd rather spend time adding copy-constructors to existing
classes to make things safe.

> If you have nothing against it I'll return to DBH "reinventing".

Ok.

Bye,

M.



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Milan Babuskov-5
In reply to this post by Michael Hieke-2
Michael Hieke wrote:
>> Mixing quoted and unquoted seems extremely error prone to me.
>
> Ok, mixing quoted and unquoted does not bother me personally, but I find
> your POV equally valid.  And this automatic decision whether to use
> quotes looks much better to me than a user option.

Mixing in what? A single statement? What if all identifiers are
uppercase (so it doesn't get quoted) and you add one that isn't? Should
we parse the current buffer for identifiers and quote them? Or do we use
this automatic quoting only in "generated" scripts? I'd still rather
have it as an SQL editor option, but I won't complain much as I don't
user lowercase/non-ascii/reserved_words identifiers, and don't quote at all.

M.


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Kjell Rilbe
Milan Babuskov wrote:

> Michael Hieke wrote:
>
>>> Mixing quoted and unquoted seems extremely error prone to me.
>>
>> Ok, mixing quoted and unquoted does not bother me personally, but I
>> find your POV equally valid.  And this automatic decision whether to
>> use quotes looks much better to me than a user option.
>
> Mixing in what? A single statement? What if all identifiers are
> uppercase (so it doesn't get quoted) and you add one that isn't? Should
> we parse the current buffer for identifiers and quote them? Or do we use
> this automatic quoting only in "generated" scripts? I'd still rather
> have it as an SQL editor option, but I won't complain much as I don't
> user lowercase/non-ascii/reserved_words identifiers, and don't quote at
> all.

Good question! :-) I repeat: I would prefer to have the global option,
but if there are already too many options in FR, I can accept to have it
as Michael suggested. And no, I don't want FR to add quotes if I edit
the script and add a quoted identifier. I *never* want an editor to
start meddling with my text while I'm editing.

Kjell
--
--------------------------------------
Kjell Rilbe
Adressmarknaden AM AB
E-post: [hidden email]
Telefon: 08-761 06 55
Mobil: 0733-44 24 64


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Michael Hieke-2
In reply to this post by Milan Babuskov-5
Milan,

Milan Babuskov wrote:

>> We should not decide whether a name needs to be quoted, but instead
>> keep the DBH object names, quoted or unquoted, as we got them -
>> either from the database, or from the user.  We should only ever
>> *add* quotes when necessary (for instance when a reserved word in
>> upper case letters is read as the column name from the database),
>> but never remove quotes.
>
> I disagree. I would hate to see a quoted name in main tree, property
> page, etc. It would also make all the sorting look dumb, as all
> quoted stuff would come first, and unquoted later. Besides, even if
> user creates an object using SQL statement with quotes, FB will
> return it unquoted back to us. Sounds like a potential for a big
> mess. IMO, we should strip quotes always, and only add them when
> needed.

Sorry, a misunderstanding.  Of course there should be no quotes in the
tree, but the name should be "as-is", meaning in exactly the case that
it was entered by the user, or read from the database.  That is no
problem with a dedicated Identifier class, as it can have several
accessor methods.  One for display, one for using in SQL statements, ...

The statement parser should return the identifier unquoted, but with
different token types for quoted and unquoted identifiers.  That way the
correct Identifier object can be created.  Right now we have only a
string, so we do not know later on whether it was quoted or not.  So I
agree, strip quotes always, but keep the information inside of the
Identifier object.

I hope that is clearer now.

>> The first fundamental change needed is the replacement of current
>>
>>   wxString nameM;
>>
>> in MetadataItem with something like
>>
>>   Identifier nameM;
>>
>> a class which has methods to compare with another Identifier /
>> wxString / std::string for equality.
>
>
> Good idea. I'll look into that direction before considering further
> steps. So, Identifier class would then have "getName" and
> "getQuotedName" methods? We wouldn't ask for MetadataItem::getName() but
> rather
>
> MetadataItem::getName()::getString()  and
> MetadataItem::getName()::getQuotedString()  or something like that?
>
> Before you comment on it again: The only place where we should see the
> quotes are the SQL scripts! That's why we need those two functions.

Agreed.  Maybe better something like

   const Identifier& MetadataItem::getIdentifier();

and a few accessors like

   const wxString& MetadataItem::getDisplayName(); // never quoted
   const wxString& MetadataItem::getName();        // quoted as necessary
   const wxString& MetadataItem::getQuotedName();  // always quoted

because MetadataItem::getName()::getFoo() isn't considered good practice?

> You should ask on the list, or me privately if not appropriate. I
> wondered why the list was so quiet.

Uh, I did.  I just haven't followed up yet on recursive locking, even
though we discussed it.

I started on clean-up of STC stuff, but more needs to be done:
- move find and replace stuff into TextControl class, decouple
TextControl and FindDialog via abstract interfaces
- create proper SqlEditor class outside of ExecuteSqlFrame, and make it
inherit from TextControl
- use LogTextControl in ExecuteSqlFrame
- ...

I looked into execution engine, but gave up.  This looks much easier
once some stuff has been moved out of ExecuteSqlFrame, and a parser is
in place.

I looked into statement parser class, but it would be much easier to
implement it if statements were std::string, using UTF-8.  Needs to be
discussed.

>> Do you really feel good with adding more code (== features) to FR,
>> without cleaning up the base first?
>
> Depends on level of their involvement with other code. I feel that I
> can easily add user management/grants, without much trouble, and
> that's my next step. Also, DDL script generator for all objects
> doesn't look too hard (if you skip this Identifier issue we discussed
> today) - a visitor for each object type and we're done (more on that
> in some other thread, later). OTOH, I do feel that stuff like
> multithreaded query executing for example, would be very hard to add
> before cleanup of ExecuteSqlFrame.

Agreed.  Maybe finishing the text control stuff is a good first step: it
seems manageable enough, and will remove some code from this behemoth ;-)

> It's just that I need some features now, so I don't have to use
> command-line tools, and I wish to implement them. I won't mind
> helping the rewrite of that code to some new architecture.

Understood.
Well, we will have time in Prague to discuss FR in greater detail :-)

Thanks

--
Michael Hieke


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

RE: RFC: Quotable names

Si Carter
In reply to this post by Kjell Rilbe
 

> -----Original Message-----
> Good question! :-) I repeat: I would prefer to have the
> global option, but if there are already too many options in
> FR, I can accept to have it as Michael suggested. And no, I
> don't want FR to add quotes if I edit the script and add a
> quoted identifier. I *never* want an editor to start meddling
> with my text while I'm editing.

+1

I never use dbl quotes, peeves me when I have to remove them when extracting
meta data.

Rgds

Si



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Michael Hieke-2
In reply to this post by Kjell Rilbe
Milan, Kjell,

Kjell Rilbe wrote:

> Milan Babuskov wrote:
>
>> Mixing in what? A single statement? What if all identifiers are
>> uppercase (so it doesn't get quoted) and you add one that isn't?

We only need to talk about FR creating statements.  Let's have this
global option "Quote all identifiers" for created statements, and if
it's not set, then FR creates either mixed or completely quoted
statements if at least one column needs to be quoted.  Again, I don't
mind mixed quotes, but I can understand that others may do.

>> Should we parse the current buffer for identifiers and quote them?
>> Or do we use this automatic quoting only in "generated" scripts?

I'd say yes.

>> I'd still rather have it as an SQL editor option, but I won't
>> complain much as I don't user lowercase/non-ascii/reserved_words
>> identifiers, and don't quote at all.

I don't use them either, and I agree with the option, no big deal.
Still, we need to decide what to do if that option is off, and at least
one name needs quotes.  I'd say it's better to quote them all or allow
for mixed statements (you decide) than to CYAO (Create Yet Another Option).

> Good question! :-) I repeat: I would prefer to have the global
> option, but if there are already too many options in FR, I can accept
> to have it as Michael suggested.

Each option for itself is not a problem, but a lot of them usually
indicates lack of care on the side of the developer.  Options should not
be used just because the decision what works best for the user is hard ;-)

> And no, I don't want FR to add quotes if I edit the script and add a
> quoted identifier. I *never* want an editor to start meddling with my
> text while I'm editing.

+1

Thanks

--
Michael Hieke


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Kjell Rilbe
Michael Hieke wrote:

> Milan, Kjell,
>
> Kjell Rilbe wrote:
>
>> Milan Babuskov wrote:
>>
>>> Mixing in what? A single statement? What if all identifiers are
>>> uppercase (so it doesn't get quoted) and you add one that isn't?
>
>
> We only need to talk about FR creating statements.  Let's have this
> global option "Quote all identifiers" for created statements, and if
> it's not set, then FR creates either mixed or completely quoted
> statements if at least one column needs to be quoted.  Again, I don't
> mind mixed quotes, but I can understand that others may do.

If there's an option to always use quotes, then I don't mind FR mixing
quoted/unquoted.

>>> Should we parse the current buffer for identifiers and quote them?
>>> Or do we use this automatic quoting only in "generated" scripts?
>
> I'd say yes.

Yes to what? The latter I hope: auto q only in gereated scripts.

So, do we have consensus:

1. Add option "Always quote identifiers".

2. When the option is off, FR quotes only identifiers that so require:
those that contain other characters than uppercase plain ascii.

3. When the option is on, FR always quotes all identifiers.

4. 2 and 3 applies to generated SQL scripts only. In all other places,
FR uses the name as it's stored in FB (but doesn't quote them. The name
MyTable woudl appear as MyTable, not "MyTable".

5. FR does not do anything at all with quotes in scripts that the user
is editing.

All agree?

Kjell
--
--------------------------------------
Kjell Rilbe
Adressmarknaden AM AB
E-post: [hidden email]
Telefon: 08-761 06 55
Mobil: 0733-44 24 64


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Milan Babuskov-5
In reply to this post by Michael Hieke-2
Michael Hieke wrote:
> I don't use them either, and I agree with the option, no big deal.
> Still, we need to decide what to do if that option is off, and at least
> one name needs quotes.  I'd say it's better to quote them all or allow
> for mixed statements (you decide) than to CYAO (Create Yet Another Option).

Ok. I would go for b) option and if someone want's everything quoted, he
has got that other option to quote everything.

>> Good question! :-) I repeat: I would prefer to have the global option,
>> but if there are already too many options in FR, I can accept
>> to have it as Michael suggested.
>
> Each option for itself is not a problem, but a lot of them usually
> indicates lack of care on the side of the developer.  Options should not
> be used just because the decision what works best for the user is hard ;-)

You're probably right. On the other hand, I believe all the options we
have so far are product of one user wanting "this" and some other user
wanting "that". If you can find some settings that we all set to same
value, I'll gladly agree to remove it.

>> And no, I don't want FR to add quotes if I edit the script and add a
>> quoted identifier. I *never* want an editor to start meddling with my
>> text while I'm editing.
>
> +1

Ok, agreed on that too.

M.



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Michael Hieke-2
In reply to this post by Kjell Rilbe
Kjell,

Kjell Rilbe wrote:

>>>> Should we parse the current buffer for identifiers and quote them?
>>>> Or do we use this automatic quoting only in "generated" scripts?
>>
>> I'd say yes.
>
> Yes to what? The latter I hope: auto q only in gereated scripts.

Yes to the latter, maybe I should have removed the first line.

And I agree completely with your summary, with the following addition:

> 2. When the option is off, FR quotes only identifiers that so
> require: those that contain other characters than uppercase plain
> ascii.

and reserved words.

Thanks

--
Michael Hieke


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Milan Babuskov-5
In reply to this post by Kjell Rilbe
Kjell Rilbe wrote:

> Michael Hieke wrote:
>
>> Milan, Kjell,
>>
>> Kjell Rilbe wrote:
>>
>>> Milan Babuskov wrote:
>>>
>>>> Mixing in what? A single statement? What if all identifiers are
>>>> uppercase (so it doesn't get quoted) and you add one that isn't?
>>
>>
>>
>> We only need to talk about FR creating statements.  Let's have this
>> global option "Quote all identifiers" for created statements, and if
>> it's not set, then FR creates either mixed or completely quoted
>> statements if at least one column needs to be quoted.  Again, I don't
>> mind mixed quotes, but I can understand that others may do.
>
>
> If there's an option to always use quotes, then I don't mind FR mixing
> quoted/unquoted.
>
>>>> Should we parse the current buffer for identifiers and quote them?
>>>> Or do we use this automatic quoting only in "generated" scripts?
>>
>>
>> I'd say yes.
>
>
> Yes to what? The latter I hope: auto q only in gereated scripts.
>
> So, do we have consensus:
>
> 1. Add option "Always quote identifiers".
>
> 2. When the option is off, FR quotes only identifiers that so require:
> those that contain other characters than uppercase plain ascii.
>
> 3. When the option is on, FR always quotes all identifiers.
>
> 4. 2 and 3 applies to generated SQL scripts only. In all other places,
> FR uses the name as it's stored in FB (but doesn't quote them. The name
> MyTable woudl appear as MyTable, not "MyTable".
>
> 5. FR does not do anything at all with quotes in scripts that the user
> is editing.
>
> All agree?

I do. That's sums it up nicely.

M.


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Kjell Rilbe
In reply to this post by Michael Hieke-2
Michael Hieke wrote:

>   const Identifier& MetadataItem::getIdentifier();
>
> and a few accessors like
>
>   const wxString& MetadataItem::getDisplayName(); // never quoted
>   const wxString& MetadataItem::getName();        // quoted as necessary
>   const wxString& MetadataItem::getQuotedName();  // always quoted

You need one accessor for SQL scripts and one for other uses. That's it.

The one for SQL script adds quotes if option "always use quotes" is on
or if the name contains other characters than uppercase plain ascii or
is equal to a reserved word. Otherwise it doesn't add quotes.

The one for other uses than SQL always just returns the name as is - no
uppercase conversion or anything.

Build the logic of quotes or not into the Identifier class - you don't
want to code a lot of if:s everywhere.

Of course, an unquoted identifier entered by the user should always be
converted to uppercase before being entered into an item object. Right?

Oh, it might be nice to have a query method requiresQuotes() on
Identifier. (Maybe on MetadatItem too?) This qould return true if the
name requires quotes. It doesn't care about the option "always use quotes".

Kjell
--
--------------------------------------
Kjell Rilbe
Adressmarknaden AM AB
E-post: [hidden email]
Telefon: 08-761 06 55
Mobil: 0733-44 24 64


-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: Quotable names

Milan Babuskov-2
In reply to this post by Michael Hieke-2
Michael Hieke wrote:

> Sorry, a misunderstanding.  Of course there should be no quotes in the
> tree, but the name should be "as-is", meaning in exactly the case that
> it was entered by the user, or read from the database.  That is no
> problem with a dedicated Identifier class, as it can have several
> accessor methods.  One for display, one for using in SQL statements, ...
>
> The statement parser should return the identifier unquoted, but with
> different token types for quoted and unquoted identifiers.  That way the
> correct Identifier object can be created.  Right now we have only a
> string, so we do not know later on whether it was quoted or not.  So I
> agree, strip quotes always, but keep the information inside of the
> Identifier object.
>
> I hope that is clearer now.

Yep, exactly as I imagined you wanted to say. ;)
So, this is agreed then.

> Agreed.  Maybe better something like
>
>   const Identifier& MetadataItem::getIdentifier();
>
> and a few accessors like
>
>   const wxString& MetadataItem::getDisplayName(); // never quoted
>   const wxString& MetadataItem::getName();        // quoted as necessary
>   const wxString& MetadataItem::getQuotedName();  // always quoted
>
> because MetadataItem::getName()::getFoo() isn't considered good practice?

Fine with me. It would keep Identifier stuff in separate class, yet
allow for simple usage.

>> You should ask on the list, or me privately if not appropriate. I
>> wondered why the list was so quiet.
>
> Uh, I did.  I just haven't followed up yet on recursive locking, even
> though we discussed it.
>
> I started on clean-up of STC stuff, but more needs to be done:
> - move find and replace stuff into TextControl class, decouple
> TextControl and FindDialog via abstract interfaces
> - create proper SqlEditor class outside of ExecuteSqlFrame, and make it
> inherit from TextControl
> - use LogTextControl in ExecuteSqlFrame
> - ...

I've seen you went that path. I didn't comment anything, as it seems
really good. Please take note that we already have
TextCtrlWithContextMenu class in MultilineEnterDialog. Pehaps that could
be mixed up in here.

> I looked into execution engine, but gave up.  This looks much easier
> once some stuff has been moved out of ExecuteSqlFrame, and a parser is
> in place.

I share that opinion.

> I looked into statement parser class, but it would be much easier to
> implement it if statements were std::string, using UTF-8.  Needs to be
> discussed.

Feel free to open new thread about it when you're ready.

> Agreed.  Maybe finishing the text control stuff is a good first step: it
> seems manageable enough, and will remove some code from this behemoth ;-)

Ok.

>> It's just that I need some features now, so I don't have to use
>> command-line tools, and I wish to implement them. I won't mind
>> helping the rewrite of that code to some new architecture.
>
> Understood.
> Well, we will have time in Prague to discuss FR in greater detail :-)

We sure will. :)

--
Milan Babuskov
http://www.flamerobin.org



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Flamerobin-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/flamerobin-devel
12