You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "gianm (via GitHub)" <gi...@apache.org> on 2024/08/05 18:39:59 UTC
[PR] SQL: Add ProjectableFilterableTable to SegmentsTable. (druid)
gianm opened a new pull request, #16841:
URL: https://github.com/apache/druid/pull/16841
This allows us to skip serialization of expensive fields such as shard_spec, dimensions, metrics, and last_compaction_state, if those fields are not actually being queried.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] SQL: Add ProjectableFilterableTable to SegmentsTable. (druid)
Posted by "georgew5656 (via GitHub)" <gi...@apache.org>.
georgew5656 merged PR #16841:
URL: https://github.com/apache/druid/pull/16841
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] SQL: Add ProjectableFilterableTable to SegmentsTable. (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #16841:
URL: https://github.com/apache/druid/pull/16841#discussion_r1704544130
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -241,7 +250,7 @@ public Map<String, Table> getTableMap()
/**
* This table contains row per segment from metadata store as well as served segments.
*/
- static class SegmentsTable extends AbstractTable implements ScannableTable
+ static class SegmentsTable extends AbstractTable implements ScannableTable, ProjectableFilterableTable
Review Comment:
Does `SegmentsTable` still need to implement `ScannableTable`? I'd think the new `scan(DataContext, List<RexNode>, int[])` function from `ProjectableFilterableTable` would be called with empty filters and null projects when projections and filters are not specified in the query
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] SQL: Add ProjectableFilterableTable to SegmentsTable. (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #16841:
URL: https://github.com/apache/druid/pull/16841#discussion_r1704544130
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -241,7 +250,7 @@ public Map<String, Table> getTableMap()
/**
* This table contains row per segment from metadata store as well as served segments.
*/
- static class SegmentsTable extends AbstractTable implements ScannableTable
+ static class SegmentsTable extends AbstractTable implements ScannableTable, ProjectableFilterableTable
Review Comment:
Does `SegmentsTable` still need to implement `ScannableTable`? I'd think the new `scan(DataContext, List<RexNode>, int[])` function from `ProjectableFilterableTable` would be called with empty filters and null projects if it's not present in the query
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] SQL: Add ProjectableFilterableTable to SegmentsTable. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16841:
URL: https://github.com/apache/druid/pull/16841#discussion_r1704680192
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -157,6 +161,11 @@ public class SystemSchema extends AbstractSchema
.add("replication_factor", ColumnType.LONG)
.build();
+ static final int SEGMENTS_SHARD_SPEC_INDEX = SEGMENTS_SIGNATURE.indexOf("shard_spec");
+ static final int SEGMENTS_DIMENSIONS_INDEX = SEGMENTS_SIGNATURE.indexOf("dimensions");
+ static final int SEGMENTS_METRICS_INDEX = SEGMENTS_SIGNATURE.indexOf("metrics");
+ static final int SEGMENTS_LAST_COMPACTION_STATE_INDEX = SEGMENTS_SIGNATURE.indexOf("last_compaction_state");
Review Comment:
I did a refactor that makes these unnecessary. The new logic moves `writeValueAsString` to `projectSegmentsRow`, and avoids both `ObjectMapper.writeValueAsString(o)` and `o.toString()` when `o` isn't being selected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] SQL: Add ProjectableFilterableTable to SegmentsTable. (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #16841:
URL: https://github.com/apache/druid/pull/16841#discussion_r1704544575
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -157,6 +161,11 @@ public class SystemSchema extends AbstractSchema
.add("replication_factor", ColumnType.LONG)
.build();
+ static final int SEGMENTS_SHARD_SPEC_INDEX = SEGMENTS_SIGNATURE.indexOf("shard_spec");
+ static final int SEGMENTS_DIMENSIONS_INDEX = SEGMENTS_SIGNATURE.indexOf("dimensions");
+ static final int SEGMENTS_METRICS_INDEX = SEGMENTS_SIGNATURE.indexOf("metrics");
+ static final int SEGMENTS_LAST_COMPACTION_STATE_INDEX = SEGMENTS_SIGNATURE.indexOf("last_compaction_state");
Review Comment:
nit: all of these can be `private static`
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -241,7 +250,7 @@ public Map<String, Table> getTableMap()
/**
* This table contains row per segment from metadata store as well as served segments.
*/
- static class SegmentsTable extends AbstractTable implements ScannableTable
+ static class SegmentsTable extends AbstractTable implements ScannableTable, ProjectableFilterableTable
Review Comment:
Do we still need to implement `ScannableTable`? I'd think the new `scan(DataContext, List<RexNode>, int[])` signature would come with empty filters and null projects if it's not present in the query
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] SQL: Add ProjectableFilterableTable to SegmentsTable. (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #16841:
URL: https://github.com/apache/druid/pull/16841#discussion_r1704544130
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -241,7 +250,7 @@ public Map<String, Table> getTableMap()
/**
* This table contains row per segment from metadata store as well as served segments.
*/
- static class SegmentsTable extends AbstractTable implements ScannableTable
+ static class SegmentsTable extends AbstractTable implements ScannableTable, ProjectableFilterableTable
Review Comment:
Do we still need to implement `ScannableTable`? I'd think the new `scan(DataContext, List<RexNode>, int[])` signature would be called with empty filters and null projects if it's not present in the query
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] SQL: Add ProjectableFilterableTable to SegmentsTable. (druid)
Posted by "abhishekrb19 (via GitHub)" <gi...@apache.org>.
abhishekrb19 commented on code in PR #16841:
URL: https://github.com/apache/druid/pull/16841#discussion_r1704544130
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -241,7 +250,7 @@ public Map<String, Table> getTableMap()
/**
* This table contains row per segment from metadata store as well as served segments.
*/
- static class SegmentsTable extends AbstractTable implements ScannableTable
+ static class SegmentsTable extends AbstractTable implements ScannableTable, ProjectableFilterableTable
Review Comment:
Does `SegmentsTable` still need to implement `ScannableTable`? I'd think the new `scan(DataContext, List<RexNode>, int[])` signature from `ProjectableFilterableTable` would be called with empty filters and null projects if it's not present in the query
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] SQL: Add ProjectableFilterableTable to SegmentsTable. (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #16841:
URL: https://github.com/apache/druid/pull/16841#discussion_r1704679657
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/SystemSchema.java:
##########
@@ -241,7 +250,7 @@ public Map<String, Table> getTableMap()
/**
* This table contains row per segment from metadata store as well as served segments.
*/
- static class SegmentsTable extends AbstractTable implements ScannableTable
+ static class SegmentsTable extends AbstractTable implements ScannableTable, ProjectableFilterableTable
Review Comment:
It seems it doesn't. I removed `ScannableTable`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org