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

Support properties system table in Hive #268

Closed
wants to merge 1 commit into from

Conversation

qqibrow
Copy link
Contributor

@qqibrow qqibrow commented Feb 20, 2019

Support Query syntax SELECT * FROM "table_name$properties" to return properties defined in hive metastore

Ref prestodb/presto#10793
Fixes prestodb/presto#10611

Copy link
Member

@kokosing kokosing left a comment

Choose a reason for hiding this comment

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

Please squash commits.

@cla-bot cla-bot bot added the cla-signed label Feb 23, 2019
@trinodb trinodb deleted a comment from cla-bot bot Feb 23, 2019
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.

A few minor nits, otherwise looks good. Per the class rename comment, please squash the commits.

Please ping me after you have addressed these so that we can get this merged.

"CREATE TABLE test_show_properties" +
" WITH (" +
"format = 'orc', " +
"partitioned_by = ARRAY[ 'SHIP_PRIORITY', 'ORDER_STATUS' ]," +
Copy link
Member

Choose a reason for hiding this comment

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

Nit: don't put spaces around array brackets. Also, use lowercase for the column names.

partitioned_by = ARRAY['ship_priority', 'order_status'],
orc_bloom_filter_columns = ARRAY['ship_priority', 'order_status'],

"FROM tpch.tiny.orders";

assertUpdate(createTable, "SELECT count(*) FROM orders");
BasicQueryInfo createTableQuery = ((DistributedQueryRunner) getQueryRunner()).getCoordinator().getQueryManager().getQueries().stream().filter(basicQueryInfo -> basicQueryInfo.getQuery().equals(createTable)).findFirst().get();
Copy link
Member

Choose a reason for hiding this comment

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

This is a clever way to get the query ID and node version! However, there is a simpler way:

String queryId = (String) computeScalar("SELECT query_id FROM system.runtime.queries WHERE query LIKE 'CREATE TABLE test_show_properties%'");
String nodeVersion = (String) computeScalar("SELECT node_version FROM system.runtime.nodes WHERE coordinator");

assertUpdate(createTable, "SELECT count(*) FROM orders");
BasicQueryInfo createTableQuery = ((DistributedQueryRunner) getQueryRunner()).getCoordinator().getQueryManager().getQueries().stream().filter(basicQueryInfo -> basicQueryInfo.getQuery().equals(createTable)).findFirst().get();
String nodeVersion = ((DistributedQueryRunner) getQueryRunner()).getCoordinator().refreshNodes().getActiveNodes().stream().map(Node::getVersion).findAny().get();
assertQuery("SELECT * FROM \"test_show_properties$properties\"", "SELECT '" + "ship_priority,order_status" + "','" + "0.5" + "','" + createTableQuery.getQueryId().getId() + "','" + nodeVersion + "'");
Copy link
Member

Choose a reason for hiding this comment

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

This is hard to read. Please format like

assertQuery(
        "SELECT * FROM \"test_show_properties$properties\"",
        format("SELECT 'ship_priority,order_status', 0.5, '%s', '%s', queryId, nodeVersion));

@electrum electrum assigned electrum and unassigned electrum Apr 22, 2019
@electrum electrum self-requested a review April 22, 2019 21:59
@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.

Access to Hive tblproperties via Hive System table <table>$properties
3 participants