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

information_schema should not fail for illegal storage format #568

Closed
wants to merge 2 commits into from

Conversation

VicoWu
Copy link
Contributor

@VicoWu VicoWu commented Mar 31, 2019

Hi @electrum @kokosing
I have just migrated the original PR(prestodb/presto#11847) from prestodb to prestosql.

@cla-bot cla-bot bot added the cla-signed label Mar 31, 2019
@VicoWu VicoWu force-pushed the hotfix-output-format-null branch 2 times, most recently from 7736f10 to fef6c3b Compare March 31, 2019 05:13
@VicoWu VicoWu force-pushed the hotfix-output-format-null branch from fef6c3b to 4beaddb Compare March 31, 2019 11:11
@VicoWu
Copy link
Contributor Author

VicoWu commented Mar 31, 2019

@electrum @kokosing
Do you have any ideas about where is this kind of table(without storageformat info) created? I try to create table in presto like create table abc( a int) , but it does have storageformat infomation;

@VicoWu
Copy link
Contributor Author

VicoWu commented Mar 31, 2019

@kokosing @electrum
I have updated the test case for it.The test logic is:
I try to create a hive table with storage format whose serde, inputformat and outputformat is null . After created , I try to retrieve this table, it should be retrieved succesfully;
Is this logic right?

@electrum
Copy link
Member

electrum commented Mar 31, 2019 via email

@kokosing
Copy link
Member

kokosing commented Apr 1, 2019

See io.prestosql.tests.utils.QueryExecutors#onHive, with this you can pass SQL to hive.

Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Please fix the build failure, then I can merge it. Thanks for fixing this issue.

@@ -108,6 +108,7 @@
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.metastore.TableType;
import org.iq80.leveldb.table.TableBuilder;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was accidentally imported and is causing the build to fail. Please remove it.

@electrum
Copy link
Member

@VicoWu Do you want to fix the build failure? Otherwise I can fix it up and merge it

@VicoWu
Copy link
Contributor Author

VicoWu commented Apr 29, 2019

@electrum The build error has been fixed. Thanks

@electrum
Copy link
Member

Thanks! Can you squash the commits?

@electrum
Copy link
Member

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants