[
https://issues.apache.org/jira/browse/NETBEANS-5831?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17373630#comment-17373630
]
Eric Bresie edited comment on NETBEANS-5831 at 7/2/21, 4:13 PM:
----------------------------------------------------------------
Key points from PR-2820 comments:
{code:java}
matthiasblaesing on Mar 21
You could create an SQL Standard Quoter
ebresie on Mar 21
how should that "Quoter" be created?
I see a case like the below using SQLIdentifer to created it, but in this case
since there is no conn available, using the meta data I don't believe would be
possible.
DatabaseMetaData dmd = conn.getMetaData();
quoter = SQLIdentifiers.createQuoter(dmd)
I see a constructor for NonASCIIQuoter(String quoteString) which extends from
Quoter, however the NonASCIIQuoter is a test class so assume that isn't usable
in this case.
matthiasblaesing on Mar 22
It depends. I was the one, that pointed out, that it is highly problematic to
try to create a completion without a corresponding connection. There is
Standard SQL, so you can build a quoter based on that, but then you also need
to guess what is considered a keyword and what not.
Maybe there is a different approach possible: Don't try to be clever and use a
parser, but use keyword based completion. You could have a look at phpmyadmin
completion how it is handled there. I doubt, that they went the way of a full
parser in JS, but most probably complete based on keyword prefix matching.
ebresie on Apr 6
Just for clarification, what is meant by "Standard SQL" in this context? Are
you talking about a specific class, a "new class", or referring to "standard
SQL syntax"?
Presently the constructor/creators of Quoters that I found was mainly the
"SqlIdentifiers.createQuoter which requires a DatabaseMetadata parameter; if
the connection is not available, then assume getting the DatabaseMetaData is
not possible. There is the "NonASCIIQuoter" but this is a test class and not in
the main codebase.
As an alternative, in that code it uses the quoter to "unquote" stuff, but if a
quoter is not available (due to lack of connection), could it just return the
text as is with something like?
if (quoter == null)
return seq.token().text().toString();
ebresie on Apr 9
Looking at one of the [w3 resources SQL syntax page in identifier section
|https://www.w3resource.com/sql/sql-syntax.php#IDENTIF] [addressing different
identifiers for different DB systems] does look more complicated.
The w3 reference mentions after "SELECT" there can be optionally "DISTINCT |
ALL" and either a wildcard ("*") or select List (i.e. identifiers). Unless I've
missed it, I'm not sure these cases are handled. Is a new issue for adding that
worth wild?
@ebresie ebresie on Apr 11
When using another tool for SQL it allows user to write without a connection
but cannot execute sql or do autocomplete without it.
@ebresie ebresie on Apr 15
Unless I'm assuming incorrectly, when no connection is active, then assume no
real need to quote/unquote any table/column would be necessary, so is going
with the below reasonable?
if (quoter == null)
return seq.token().text().toString();
@matthiasblaesing matthiasblaesing on Apr 15
My pain point: Instead of trying to fix logic, that relies hard on a
connection, why not refactor this to split connection less and connction
required codepaths, then it would be clear and you'd not need to unquote.
@ebresie ebresie on Apr 16 Author Contributor
Matthias, I appreciate the feedback. As a new commiter, I know I;m not there
yet but will get there with help.
The original scope of this ticket was...if no connection, allow some form of
auto completion without connection based details and avoid the connection
dialog from preventing futher autocompletion. As it is now I think it does this.
The remaining concenrs is how getUnquotedIdentifier behaves when no connection
is available. Initial changes was to returned "", which I realize is wrong,
risking the loss of given seq.token.txt details. Next change was to return the
results without being "unquoted" with given "return
seq.token().text().toString();"; whether it needed or not was dependent on the
database in use which if not available is where problems may occur. When quoter
is not available or used avoids risk of a null pointers and return the sequence
of text in the given context without being concerned without being "unquoted"
independent of whether a "quoter" was available or not.
When quoter is available and calls the "unquote" method found in the
SQLIdentifer.Quoter abstract class which is implemented/extended in the
DatabaseMetaDataQuoter implementation; if quoter was set correctly previously
(when DB is available) then instance of quoter should be available and all is
right with the world. However, when quoter is unavailable (not connection and
not setup earlier) then something different is needed.
So is the change suggetion to implement a new ConnectionLessQuoter or
BasicQuoter under the SqlIdentifer class and use this when connection is not
available earlier in the flow?
It seems many of the methods in the Quoter abstract class probably should be
static methods. So is the refactoring mentioned involve moving some of these
out into a "static" context in some way?
As implemented, "getUnquotedIdentifer" is used in placed like
"InsertStatementAnalyzer, SQLStatemenAnalyzer, and SelectStatementAnalyzer,
analyzeChosenColumns, etc. In other places, some form of interface is used like
"TablesClause" (assume this would be "connection based") used in places such as
"DeleteStatementAnalyzer", InsertStatementAnalyzer, SelectStatementAnalyzer,
and UpdateStatementAnalyzer". Or uses a call to "parseIdentifier" which also
use the getUnquotedIdentifer or parseTableIdent to return table names
"Identifiers". So are changes to standardize these across context needed here?
For SqlStatementAnalyizer, the "connection" state is not know directly, but is
available previously such as in the SQLCompletionQuery context where it
established the quoter and passes forward to applicable as. So in a given
Analyzer context, the "quoter" was the way to determine if there was or wasn't
a connection without passing around the connections from other scope/context.
Do this type of aspect reflect the "connection vs non-connection" based changes
preferred in earlier comment?
Connection may be needed when determining what kind of identifiers to include
in the autocomplete options in applicable context of a given sql statement
(i.e. between select xxx from, insert .. INTO xxxx, etc.) but not others (i.e.
empty line so use initial SQL keywords). WHen available then the identifier
such as table and columns identifiers based on the connection are possible
which I bleive depneds on connected based DatabaseMetaDataQuoter and related
classed. This may help determine if should or shouldn't quote given identifiers
(database/schema/table/column) for given DB product. If connection is not
available, then maybe able to add "non connection based identifier items" (i.e.
"", Count(), DISTINCT, etc. - not sure it does this presently; is this where a
StandardSQLQuoter might come in to lay?).
In each call to "getUnquotedIdentifer()" it return a list Strings with possible
items. It iterates through each token in the current expression and may key on
before/after "dot" or if typing an identifier make another call. In other cases
it may use the above mentioned "createTable" type style so
Can't really create a DB quoter without a give connection; and no easy way to
determine what connection to use without user interaction (i.e. selecting DB
connection if available, creating a connection, etc.). Some sort of "default"
connection (probably set in the connection pulldown) could be persisted in some
way and reused after it was first established to reduce the chance it's not
setup, but that also seems like a different type of change (i.e. maybe a new
"store selected connection and reused next time file or SQL editor is opened"
type ticket).
{code}
was (Author: ebresie):
Key points from PR-2820 comments:
matthiasblaesing on Mar 21
You could create an SQL Standard Quoter
ebresie on Mar 21
how should that "Quoter" be created?
I see a case like the below using SQLIdentifer to created it, but in this case
since there is no conn available, using the meta data I don't believe would be
possible.
DatabaseMetaData dmd = conn.getMetaData();
quoter = SQLIdentifiers.createQuoter(dmd)
I see a constructor for NonASCIIQuoter(String quoteString) which extends from
Quoter, however the NonASCIIQuoter is a test class so assume that isn't usable
in this case.
matthiasblaesing on Mar 22
It depends. I was the one, that pointed out, that it is highly problematic to
try to create a completion without a corresponding connection. There is
Standard SQL, so you can build a quoter based on that, but then you also need
to guess what is considered a keyword and what not.
Maybe there is a different approach possible: Don't try to be clever and use a
parser, but use keyword based completion. You could have a look at phpmyadmin
completion how it is handled there. I doubt, that they went the way of a full
parser in JS, but most probably complete based on keyword prefix matching.
ebresie on Apr 6
Just for clarification, what is meant by "Standard SQL" in this context? Are
you talking about a specific class, a "new class", or referring to "standard
SQL syntax"?
Presently the constructor/creators of Quoters that I found was mainly the
"SqlIdentifiers.createQuoter which requires a DatabaseMetadata parameter; if
the connection is not available, then assume getting the DatabaseMetaData is
not possible. There is the "NonASCIIQuoter" but this is a test class and not in
the main codebase.
As an alternative, in that code it uses the quoter to "unquote" stuff, but if a
quoter is not available (due to lack of connection), could it just return the
text as is with something like?
if (quoter == null)
return seq.token().text().toString();
ebresie on Apr 9
Looking at one of the [w3 resources SQL syntax page in identifier section
|https://www.w3resource.com/sql/sql-syntax.php#IDENTIF] [addressing different
identifiers for different DB systems] does look more complicated.
The w3 reference mentions after "SELECT" there can be optionally "DISTINCT |
ALL" and either a wildcard ("*") or select List (i.e. identifiers). Unless I've
missed it, I'm not sure these cases are handled. Is a new issue for adding that
worth wild?
@ebresie ebresie on Apr 11
When using another tool for SQL it allows user to write without a connection
but cannot execute sql or do autocomplete without it.
@ebresie ebresie on Apr 15
Unless I'm assuming incorrectly, when no connection is active, then assume no
real need to quote/unquote any table/column would be necessary, so is going
with the below reasonable?
if (quoter == null)
return seq.token().text().toString();
@matthiasblaesing matthiasblaesing on Apr 15
My pain point: Instead of trying to fix logic, that relies hard on a
connection, why not refactor this to split connection less and connction
required codepaths, then it would be clear and you'd not need to unquote.
@ebresie ebresie on Apr 16 Author Contributor
Matthias, I appreciate the feedback. As a new commiter, I know I;m not there
yet but will get there with help.
The original scope of this ticket was...if no connection, allow some form of
auto completion without connection based details and avoid the connection
dialog from preventing futher autocompletion. As it is now I think it does this.
The remaining concenrs is how getUnquotedIdentifier behaves when no connection
is available. Initial changes was to returned "", which I realize is wrong,
risking the loss of given seq.token.txt details. Next change was to return the
results without being "unquoted" with given "return
seq.token().text().toString();"; whether it needed or not was dependent on the
database in use which if not available is where problems may occur. When quoter
is not available or used avoids risk of a null pointers and return the sequence
of text in the given context without being concerned without being "unquoted"
independent of whether a "quoter" was available or not.
When quoter is available and calls the "unquote" method found in the
SQLIdentifer.Quoter abstract class which is implemented/extended in the
DatabaseMetaDataQuoter implementation; if quoter was set correctly previously
(when DB is available) then instance of quoter should be available and all is
right with the world. However, when quoter is unavailable (not connection and
not setup earlier) then something different is needed.
So is the change suggetion to implement a new ConnectionLessQuoter or
BasicQuoter under the SqlIdentifer class and use this when connection is not
available earlier in the flow?
It seems many of the methods in the Quoter abstract class probably should be
static methods. So is the refactoring mentioned involve moving some of these
out into a "static" context in some way?
As implemented, "getUnquotedIdentifer" is used in placed like
"InsertStatementAnalyzer, SQLStatemenAnalyzer, and SelectStatementAnalyzer,
analyzeChosenColumns, etc. In other places, some form of interface is used like
"TablesClause" (assume this would be "connection based") used in places such as
"DeleteStatementAnalyzer", InsertStatementAnalyzer, SelectStatementAnalyzer,
and UpdateStatementAnalyzer". Or uses a call to "parseIdentifier" which also
use the getUnquotedIdentifer or parseTableIdent to return table names
"Identifiers". So are changes to standardize these across context needed here?
For SqlStatementAnalyizer, the "connection" state is not know directly, but is
available previously such as in the SQLCompletionQuery context where it
established the quoter and passes forward to applicable as. So in a given
Analyzer context, the "quoter" was the way to determine if there was or wasn't
a connection without passing around the connections from other scope/context.
Do this type of aspect reflect the "connection vs non-connection" based changes
preferred in earlier comment?
Connection may be needed when determining what kind of identifiers to include
in the autocomplete options in applicable context of a given sql statement
(i.e. between select xxx from, insert .. INTO xxxx, etc.) but not others (i.e.
empty line so use initial SQL keywords). WHen available then the identifier
such as table and columns identifiers based on the connection are possible
which I bleive depneds on connected based DatabaseMetaDataQuoter and related
classed. This may help determine if should or shouldn't quote given identifiers
(database/schema/table/column) for given DB product. If connection is not
available, then maybe able to add "non connection based identifier items" (i.e.
"", Count(), DISTINCT, etc. - not sure it does this presently; is this where a
StandardSQLQuoter might come in to lay?).
In each call to "getUnquotedIdentifer()" it return a list Strings with possible
items. It iterates through each token in the current expression and may key on
before/after "dot" or if typing an identifier make another call. In other cases
it may use the above mentioned "createTable" type style so
Can't really create a DB quoter without a give connection; and no easy way to
determine what connection to use without user interaction (i.e. selecting DB
connection if available, creating a connection, etc.). Some sort of "default"
connection (probably set in the connection pulldown) could be persisted in some
way and reused after it was first established to reduce the chance it's not
setup, but that also seems like a different type of change (i.e. maybe a new
"store selected connection and reused next time file or SQL editor is opened"
type ticket).
> Create a SQL Standard Quoter for Use with Connectionless Cases
> --------------------------------------------------------------
>
> Key: NETBEANS-5831
> URL: https://issues.apache.org/jira/browse/NETBEANS-5831
> Project: NetBeans
> Issue Type: Improvement
> Components: db - SQL Editor
> Affects Versions: 12.4
> Reporter: Eric Bresie
> Priority: Minor
> Labels: sql
>
> Based on "NETBEANS-189 SQL editor, shouldn't ask evertime to set the
> connection", it was found when connectionless case occurs, this prevented
> auto completion because it relies on connection (DB metadata) based class to
> return back identifiers which may require further "quoting" to accomidate
> different DB vendor quotring behavior. When connection is not available, the
> db metadata based class does not provide back any usable token for use in
> auto completion and more specifically when quoting these identifier tokens.
> Initial changes in PR-2820 handle specific cases to handled null quoter (due
> to lack of a connection) where applicable to allow autocompletion without
> connection to occur. As part of the review of PR, It was suggested to
> refactor some of this code to develop "ConnectionLess" functionality to
> better isolate the connection vs connectionless functionality. This may
> involve development of a Standard SQL Quoter to accomidate basic SQL quoter
> logic not dependent on a connection (and metadata based identifiers).
> From NETBEANS-189 comments:
> {code:java}
> (6) Potentially larger architectural change may be needed to allow for
> Connection or Connectionless cases involving changes (rather than case by
> case changes in a number of places), which may involve introducing a new
> "Standard SQL Quoter" class for use when connection is not available instead
> of the Metadata (from the connection based handling of identifiers) which I
> suggest needs a separate ticket to create a Standard SQL Quoter class and
> retrofit the code as part of that work.
> The Standard SQl Quoter may involve standardized quoting of identifiers
> similar to what is discussed here
> https://www.w3resource.com/sql/sql-syntax.php#IDENTIF ). For this, would need
> to better understand what the expected input/outputs are for this and what
> kind of tests would be needed to confirm this does as expected beyond the
> existing sql tests.
> (7) There may be another SQL improvements ticket to raise to account for
> additional possible missing autocomplete tokens like what is listed in the w3
> reference above which mentions after "SELECT" there can be optionally
> "DISTINCT | ALL" and either a wildcard ("*"). This is different from
> selection list of possible connection based identifiers.
> {code}
> Reference:
> # https://issues.apache.org/jira/browse/NETBEANS-189
> # https://github.com/apache/netbeans/pull/2820
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@netbeans.apache.org
For additional commands, e-mail: commits-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
