Skip to content
This repository has been archived by the owner on Jan 28, 2021. It is now read-only.

Improve error on nested query without aliasing #376

Closed
campoy opened this issue Sep 21, 2018 · 11 comments · Fixed by src-d/gitbase#502
Closed

Improve error on nested query without aliasing #376

campoy opened this issue Sep 21, 2018 · 11 comments · Fixed by src-d/gitbase#502
Assignees
Labels
enhancement New feature or request

Comments

@campoy
Copy link

campoy commented Sep 21, 2018

The following query fails with a syntax error syntax error at position 54:

select repository_id, ref_name
from (
    select * from refs
);

When adding sort the error is a bit more clear unknown error: syntax error at position 62 near 'by':

select repository_id, ref_name
from (
    select * from refs
)
sort by repository_id;

The way to fix these is to use an alias for the nested query:

select repository_id, ref_name
from (
    select * from refs
) as t;

This seems like a syntax bug?

@ajnavarro
Copy link
Contributor

ajnavarro commented Sep 24, 2018

The mysql error is more clear: Every derived table must have its own alias.

The first query is a not valid query, but the error is not clear. We are using the Vitess parser. Maybe in newer version the error is more clear.

@ajnavarro ajnavarro added the enhancement New feature or request label Sep 24, 2018
@campoy
Copy link
Author

campoy commented Sep 24, 2018

I guess I'm also ok with that error message if it's a MySQL standard requirement.

@ajnavarro ajnavarro changed the title Support nested query without aliasing the result Improve error on nested query without aliasing Sep 26, 2018
@kuba-- kuba-- self-assigned this Sep 28, 2018
@kuba--
Copy link
Contributor

kuba-- commented Sep 28, 2018

Unfortunately new vitess doesn't help in this case. We still directly get:

syntax error at position 41

from the yacc parser.

@erizocosmico
Copy link
Contributor

Is there anything we can actually do to improve this error without manually parsing (or improving the error message on vitess)? If the error vitess' parser returns is this, there's not much we can do on our side.

@ajnavarro
Copy link
Contributor

@erizocosmico I don't think so...

Closing issue.

@kuba--
Copy link
Contributor

kuba-- commented Sep 28, 2018

Actually I was able to hack vitess, so this query may work ;)
But as you said most likely it will not follow mysql standard.

@erizocosmico
Copy link
Contributor

@kuba-- but the query is incorrect, at least for mysql.

@kuba--
Copy link
Contributor

kuba-- commented Sep 28, 2018

@erizocosmico - yeah, I know ^^^^

....it will not follow mysql standard.

Just for curiosity ...
In this section: https://github.com/src-d/go-vitess/blob/bd7bf249f0771d81bb6fc29b9540e613dd2e3a35/vt/sqlparser/sql.y#L1839
I added:

| subquery
  {
    $$ = &AliasedTableExpr{Expr:$1, As: NewTableIdent("dual")}
  }

and generated a new sql parser goyacc -o sql.go sql.y

I was able to query:

	_, iter, err := e.Query(newCtx(), `SELECT SUM(i) FROM (select i from mytable)`)
//...
	require.Equal([]sql.Row{{float64(6)}}, rows)
}

@kuba--
Copy link
Contributor

kuba-- commented Sep 28, 2018

But if instead of workaround I'll add to sql.y

| subquery
  {
    // missed alias for subquery
    yylex.Error("Missed alias for subquery")
    return 1
  }

then we may get a nicer error:

        	Error Trace:	engine_test.go:1095
        	Error:      	Received unexpected error:
        	            	Missed alias for subquery at position 41
        	Test:       	TestSubquery

So maybe it's worth to send yet another PR to vitess
WDYT?

@ajnavarro
Copy link
Contributor

Not in all cases a subquery needs an alias.

@kuba--
Copy link
Contributor

kuba-- commented Sep 28, 2018

Related: vitessio/vitess#4222

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants