Add TableStatistics in QueryInputMetadata for Join Query #12808
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
viczhang861
changed the title
arhimondr
approved these changes
May 17, 2019
| @@ -35,26 +36,23 @@ | |||
| private final String table; | |||
| private final List<Column> columns; | |||
| private final Optional<Object> connectorInfo; | |||
| private final Optional<TableStatistics> tableStats; | |||
| @@ -58,15 +63,15 @@ private static Column createColumn(ColumnMetadata columnMetadata) | |||
| return new Column(columnMetadata.getName(), columnMetadata.getType().toString()); | |||
| } | |||
|
|
|||
| private Input createInput(TableMetadata table, TableHandle tableHandle, Set<Column> columns) | |||
| private Input createInput(TableMetadata table, TableHandle tableHandle, Set<Column> columns, Optional<TableStatistics> tableStats) | |||
| @@ -85,13 +114,20 @@ public Void visitTableScan(TableScanNode node, Void context) | |||
| columns.add(createColumn(metadata.getColumnMetadata(session, tableHandle, columnHandle))); | |||
| } | |||
|
|
|||
| inputs.add(createInput(metadata.getTableMetadata(session, tableHandle), tableHandle, columns)); | |||
| Optional<TableStatistics> tableStats = Optional.empty(); | |||
| Optional<TableStatistics> tableStats = Optional.empty(); | ||
|
|
||
| if (context.extractStats()) { | ||
| Constraint<ColumnHandle> constraint = new Constraint<>(node.getCurrentConstraint()); |
|
|
||
| private static class Context | ||
| { | ||
| private boolean extractStats; |
| @@ -78,6 +80,7 @@ public int hashCode() | |||
| return Objects.hash(min, max); | |||
| } | |||
|
|
|||
| @JsonValue | |||
There was a problem hiding this comment.
Let's annotate getMin and getMax with the @JsonProperty to keep this class Json parsable
arhimondr
reviewed
May 19, 2019
|
|
||
| if (context.extractStats()) { | ||
| Constraint<ColumnHandle> constraint = new Constraint<>(node.getCurrentConstraint()); | ||
| tableStats = Optional.of(metadata.getTableStatistics(session, tableHandle, constraint)); |
There was a problem hiding this comment.
Let's log the stats only for the columns references in the table scan
viczhang861
force-pushed
the
stats
branch
2 times, most recently
from
d41d3a1
to
ae9ede6
Compare
arhimondr
reviewed
May 21, 2019
| @@ -589,6 +589,23 @@ public TableStatistics getTableStatistics(ConnectorSession session, ConnectorTab | |||
| return hiveStatisticsProvider.getTableStatistics(session, ((HiveTableHandle) tableHandle).getSchemaTableName(), columns, columnTypes, partitions); | |||
| } | |||
|
|
|||
| @Override | |||
| public TableStatistics getTableStatistics(ConnectorSession session, ConnectorTableHandle tableHandle, Constraint<ColumnHandle> constraint, Set<ColumnHandle> desiredColumns) | |||
There was a problem hiding this comment.
Let's don't change the SPI. You can simply request the stats using the old SPI, but before storing them in the Input, filter out the stats that are not referenced in the TableScanNode recreating the TableStatistics object.
There was a problem hiding this comment.
Requesting stats for only the columns needed requires more reworking in hive connector. By asking of filtering the stats for not used columns i just wanted to optimize the storage for the Input objects.
|
Rebase |
arhimondr
approved these changes
Jun 19, 2019
presto-main/src/main/java/com/facebook/presto/sql/planner/InputExtractor.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/com/facebook/presto/sql/planner/InputExtractor.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/com/facebook/presto/sql/planner/InputExtractor.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/com/facebook/presto/sql/planner/InputExtractor.java
Outdated
Show resolved
Hide resolved
presto-main/src/main/java/com/facebook/presto/sql/planner/InputExtractor.java
Outdated
Show resolved
Hide resolved
rschlussel
approved these changes
Jun 20, 2019
arhimondr
approved these changes
Jun 20, 2019
Record table statistics in QueryCompletedEvent for join query
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.
Table stats availablity is important for cost-based optimization
of join query. Add it to QueryInputMetadata class.
Test is added to show example of TableStatisitcs in json format.