[
https://issues.apache.org/jira/browse/CASSANDRA-18302?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17699878#comment-17699878
]
Caleb Rackliffe edited comment on CASSANDRA-18302 at 3/13/23 11:30 PM:
-----------------------------------------------------------------------
Started looking through the patch, but I want to stop and talk through some
high-level things here.
The proximate cause of the original problem is that {{asCQL()}} seems to want
to call {{queriedColumns()}} to get the {{SELECT}} clause to show the user. If
you just use the {{ColumnFilter}} already calculated from the actual
`ClientState`, I think that just works:
{noformat}
public String asCQL(QueryOptions options, ClientState state)
{
ColumnFilter columnFilter =
selection.newSelectors(options).getColumnFilter();
StringBuilder sb = new StringBuilder();
sb.append("SELECT ").append(columnFilter.toCQLString());
...
{noformat}
(Side note: It looks like there are a couple places where {{asCQL()}} could be
called lazily and it isn't?)
CC [~dcapwell]
The other thing we're driving at here is making the error message as useful as
possible. Toward that end, we have line and character numbers, as well as raw
query text being propagated and filtered in the patch. What if, as a compromise
proposal, we 1.) fix {{asCQL()}} (which is used other places like
{{maybeWarn()}}) to avoid our immediate issue and 2.) pare down the patch to
just track the most important bit of the enhanced error message: the line
number?
My basic arguments are that 1.) even if they're not perfect, the statements
(lazily!) generated by {{asCQL()}} are useful enough and avoid having to add
all the new fragment extraction logic, 2.) the line number is usually all we
need for properly formatted transactions, and 3.) the character position isn't
actually that helpful for a poorly formatted transaction w/ multiple
expressions.
WDYT?
was (Author: maedhroz):
Started looking through the patch, but I want to stop and talk through some
high-level things here.
The proximate cause of the original problem is that {{asCQL()}} seems to want
to call {{queriedColumns()}} to get the {{SELECT}} clause to show the user. If
you just use the {{ColumnFilter}} already calculated from the actual
`ClientState`, I think that just works:
{noformat}
public String asCQL(QueryOptions options, ClientState state)
{
ColumnFilter columnFilter =
selection.newSelectors(options).getColumnFilter();
StringBuilder sb = new StringBuilder();
sb.append("SELECT ").append(columnFilter.toCQLString());
...
{noformat}
(Side note: It looks like there are a couple places where {{asCQL()}} could be
called lazily and it isn't?)
CC [~dcapwell]
The other thing we're driving at here is making the error message as useful as
possible. Toward that end, we have line and character numbers, as well as raw
query text being propagated and filtered in the patch. What if, as a compromise
proposal, we 1.) fix {{asCQL()}} (which is used other places like
{{maybeWarn()}}) to avoid our immediate issue and 2.) pare down the patch a
tiny bit to just track the most important bit of the enhanced error message:
the line number?
My basic arguments are that 1.) even if they're not perfect, the statements
(lazily!) generated by {{asCQL()}} are useful enough and avoid having to add
all the new fragment extraction logic, 2.) the line number is usually all we
need for properly formatted transactions, and 3.) the character position isn't
actually that helpful for a poorly formatted transaction w/ multiple
expressions.
WDYT?
> Fix AIOOBE and improve validation messages for transaction statements
> ---------------------------------------------------------------------
>
> Key: CASSANDRA-18302
> URL: https://issues.apache.org/jira/browse/CASSANDRA-18302
> Project: Cassandra
> Issue Type: Improvement
> Components: Accord
> Reporter: Jacek Lewandowski
> Assignee: Jacek Lewandowski
> Priority: Normal
> Fix For: 5.x
>
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> Currently it happens sometimes that ArrayIndexOutOfBoundsException is thrown
> from asCql method when validation transaction statement. In addition, asCql
> does not return precisely the query user entered so the whole error message
> can be misleading.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
