Fix SHOW TABLES for hidden tpch and tpcds schemas #1007
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
findepi
reviewed
Jun 15, 2019
| @Override | ||
| public boolean schemaExists(ConnectorSession session, String schemaName) | ||
| { | ||
| return schemaNameToScaleFactor(schemaName) < 0 ? false : true; |
There was a problem hiding this comment.
- sf0 does not exists
- this needs to be handled the same way as in
io.prestosql.plugin.tpcds.TpcdsMetadata#getTableHandle
Suggested change
| return schemaNameToScaleFactor(schemaName) < 0 ? false : true; | |
| return schemaNameToScaleFactor(schemaName) > 0; |
| @Override | ||
| public boolean schemaExists(ConnectorSession session, String schemaName) | ||
| { | ||
| return schemaNameToScaleFactor(schemaName) < 0 ? false : true; |
There was a problem hiding this comment.
TPCH has a bug -- io.prestosql.plugin.tpch.TpchMetadata#getTableHandle accepts scale factor 0, and so
select count(*) from tpch.sf0.nation;
- should fail with schema not found
- but it fails with:
java.lang.IllegalArgumentException: Scale factor must be larger than 0
at com.google.common.base.Preconditions.checkArgument(Preconditions.java:141)
at io.prestosql.plugin.tpch.TpchTableHandle.<init>(TpchTableHandle.java:46)
at io.prestosql.plugin.tpch.TpchTableHandle.<init>(TpchTableHandle.java:36)
at io.prestosql.plugin.tpch.TpchMetadata.getTableHandle(TpchMetadata.java:176)
So:
- fix io.prestosql.plugin.tpch.TpchMetadata#getTableHandle's
if (scaleFactoras it is in tpcds - make this line the same as in tpcds
presto-tpch/src/test/java/io/prestosql/plugin/tpch/TestTpchMetadata.java
Show resolved
Hide resolved
Praveen2112
force-pushed
the
tpch_tpcds_hidden_schemas
branch
from
53df09a
to
fa6a179
Compare
|
@findepi Thanks for your insights. Have applied the comments |
findepi
reviewed
Jun 17, 2019
|
|
||
| public void testShowTables() | ||
| { | ||
| Session session = testSessionBuilder() |
There was a problem hiding this comment.
- inline (it's used only once)
- create a helper method that constructs a Session for given schema
| if (scaleFactor < 0) { | ||
| return null; | ||
| if (scaleFactor > 0) { | ||
| return new TpchTableHandle(tableName.getTableName(), scaleFactor); |
There was a problem hiding this comment.
please revert.
Just make the comparison:
if (scaleFactor <= 0) {
return null;
}
| if (scaleFactor <= 0) { | ||
| return null; | ||
| if (scaleFactor > 0) { | ||
| return new TpcdsTableHandle(tableName.getTableName(), scaleFactor); |
| @Test | ||
| public void testShowTables() | ||
| { | ||
| Session session = testSessionBuilder() |
Praveen2112
force-pushed
the
tpch_tpcds_hidden_schemas
branch
from
fa6a179
to
8ef6c7f
Compare
findepi
approved these changes
Jul 2, 2019
|
Merged, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1005