Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

!dropall doesn't work in master branch #248

Closed
swaroopak opened this issue Jan 3, 2019 · 7 comments
Closed

!dropall doesn't work in master branch #248

swaroopak opened this issue Jan 3, 2019 · 7 comments

Comments

@swaroopak
Copy link

if (!sqlLine.getLineReader().readLine(question).equals("y")) {

!dropall doesn't drop any table. This feature works fine in 1.2.0.

@julianhyde
Copy link
Owner

I confirm that it is broken. Here's what happened when I tried to run it:

$ ./sqlline -u jdbc:calcite: -n scott -p tiger
SLF4J: Class path contains multiple SLF4J bindings.
SLF4J: Found binding in [jar:file:/Users/julianhyde/.m2/repository/org/slf4j/slf4j-log4j12/1.7.25/slf4j-log4j12-1.7.25.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: Found binding in [jar:file:/Users/julianhyde/.m2/repository/ch/qos/logback/logback-classic/1.1.3/logback-classic-1.1.3.jar!/org/slf4j/impl/StaticLoggerBinder.class]
SLF4J: See http://www.slf4j.org/codes.html#multiple_bindings for an explanation.
SLF4J: Detected both log4j-over-slf4j.jar AND slf4j-log4j12.jar on the class path, preempting StackOverflowError. 
SLF4J: See also http://www.slf4j.org/codes.html#log4jDelegationLoop for more details.
Transaction isolation level TRANSACTION_REPEATABLE_READ is not supported. Default (TRANSACTION_NONE) will be used instead.
sqlline version 1.6.0
0: jdbc:calcite:> !dropall
Really drop every table in the database? (y/n)abort-drop-all: Aborting drop all tables.y
. . . .semicolon> ;
abort-drop-all
0: jdbc:calcite:> !quit
Closing: org.apache.calcite.jdbc.CalciteJdbc41Factory$CalciteJdbc41Connection

@snuyanzin
Copy link
Collaborator

snuyanzin commented Jan 10, 2019

It looks like an issue with SqlLineParser while reading user answer. The same was while !manual command.
Here it is the possible fix in the same way as for !manual 5876b2f

However there is another issue (could be seen while running sqlline.SqlLineArgsTest#testDropAllH2).
sqlline generates queries like DROP TABLE <TABLE_NAME>; for all tables without paying attention to schema_names and H2 answers like

org.h2.jdbc.JdbcSQLException: Table "CATALOGS" not found; SQL statement:
DROP TABLE CATALOGS [42102-197]

So at least schema name should be included here e.g. DROP TABLE INFORMATION_SCHEMA.CATALOGS;
And the answer from H2 will relate to drop rather than table existence

org.h2.jdbc.JdbcSQLException: Cannot drop table "CATALOGS"; SQL statement:
DROP TABLE INFORMATION_SCHEMA.CATALOGS [90118-197]

What is the expected behavior? Should it generate queries including schemas or should !dropall take a schema_name as a parameter or whatever?

@swaroopak
Copy link
Author

@snuyanzin : Is this change committed in master? How can use this?

@snuyanzin
Copy link
Collaborator

No I have not committed it to master (it exists only in my fork), btw I do not have rights to commit it here.

However the main reason why I even have not made a PR is my previous comment.
I.e. the changes I made allow user to submit answers like (y/n) but db engine itself could not handle sqlline commands without schema names (I tried with H2 and HSQLDB).

So !dropall will not work. My suggestion here is to add schema_name argument like

!dropall [schema_name]*

If no arguments are specified then the behavior as it was before. If the schema_name or schema_names specified then sqlline will try to drop tables only from the mentioned schemas.

Please let me know if there are any objections/ideas/improvements

@snuyanzin
Copy link
Collaborator

Is this change committed in master? How can use this?

I submitted a PR which extracts logic you need in a separate method. However this PR does not fix the current issue because of open questions in my previous comment

@swaroopak
Copy link
Author

swaroopak commented Jan 11, 2019

Thank you! I'll update my PR as yours gets merged.

@snuyanzin
Copy link
Collaborator

Merged as 0be1ddd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants