You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/01/21 18:59:31 UTC
[GitHub] [druid] gianm opened a new pull request #9235: Add join-related
DataSource types, and analysis functionality.
gianm opened a new pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235
Builds on #9111 and implements the datasource analysis mentioned in #8728. Still can't
handle join datasources, but we're a step closer.
Join-related DataSource types:
1) Add "join", "lookup", and "inline" datasources.
2) Add "getChildren" and "withChildren" methods to DataSource, which will be used
in the future for query rewriting (e.g. inlining of subqueries).
DataSource analysis functionality:
1) Add DataSourceAnalysis class, which breaks down datasources into three components:
outer queries, a base datasource (left-most of the highest level left-leaning join
tree), and other joined-in leaf datasources (the right-hand branches of the
left-leaning join tree).
2) Add "isConcrete", "isGlobal", and "isCacheable" methods to DataSource in order to
support analysis.
Other notes:
1) Renamed DataSource#getNames to DataSource#getTableNames, which I think is clearer.
Also, made it a Set, so implementations don't need to worry about duplicates.
2) The addition of "isCacheable" should work around #8713, since UnionDataSource now
returns false for cacheability.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369350676
##########
File path: server/src/main/java/org/apache/druid/client/CacheUtil.java
##########
@@ -99,7 +99,8 @@
return QueryContexts.isUseCache(query)
&& strategy != null
&& cacheConfig.isUseCache()
- && cacheConfig.isQueryCacheable(query);
+ && cacheConfig.isQueryCacheable(query)
+ && query.getDataSource().isCacheable();
Review comment:
Or maybe not combining them in this patch, but at least doing the extraction.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369322015
##########
File path: processing/src/test/java/org/apache/druid/query/planning/DataSourceAnalysisTest.java
##########
@@ -0,0 +1,476 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.planning;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.query.DataSource;
+import org.apache.druid.query.InlineDataSource;
+import org.apache.druid.query.JoinDataSource;
+import org.apache.druid.query.LookupDataSource;
+import org.apache.druid.query.QueryDataSource;
+import org.apache.druid.query.TableDataSource;
+import org.apache.druid.query.UnionDataSource;
+import org.apache.druid.query.groupby.GroupByQuery;
+import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
+import org.apache.druid.segment.column.ValueType;
+import org.apache.druid.segment.join.JoinConditionAnalysis;
+import org.apache.druid.segment.join.JoinType;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Optional;
+
+public class DataSourceAnalysisTest
+{
+ private static final TableDataSource TABLE_FOO = new TableDataSource("foo");
+ private static final TableDataSource TABLE_BAR = new TableDataSource("bar");
+ private static final LookupDataSource LOOKUP_LOOKYLOO = new LookupDataSource("lookyloo");
+ private static final InlineDataSource INLINE = InlineDataSource.fromIterable(
+ ImmutableList.of("column"),
+ ImmutableList.of(ValueType.STRING),
+ ImmutableList.of(new Object[0])
+ );
+
+ @Test
+ public void testTable()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(TABLE_FOO);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(TABLE_FOO, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(unionDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(unionDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnTable()
+ {
+ final QueryDataSource queryDataSource = subquery(TABLE_FOO);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final QueryDataSource queryDataSource = subquery(unionDataSource);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testLookup()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(LOOKUP_LOOKYLOO);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnLookup()
+ {
+ final QueryDataSource queryDataSource = new QueryDataSource(
+ GroupByQuery.builder()
+ .setDataSource(LOOKUP_LOOKYLOO)
+ .setInterval(new MultipleIntervalSegmentSpec(Collections.singletonList(Intervals.of("2000/3000"))))
+ .setGranularity(Granularities.ALL)
+ .build()
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testInline()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(INLINE);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(INLINE, analysis.getDataSource());
+ Assert.assertEquals(INLINE, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testJoinSimpleLeftLeaning()
+ {
+ // Join of a table onto a variety of simple joinable objects (lookup, inline, subquery) with a left-leaning
+ // structure (no right children are joins themselves).
+
+ final JoinDataSource joinDataSource =
+ join(
+ join(
+ join(
+ TABLE_FOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ ),
+ INLINE,
+ "2.",
+ JoinType.LEFT
+ ),
+ subquery(LOOKUP_LOOKYLOO),
+ "3.",
+ JoinType.FULL
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1.")),
+ new PreJoinableClause("2.", INLINE, JoinType.LEFT, joinClause("2.")),
+ new PreJoinableClause("3.", subquery(LOOKUP_LOOKYLOO), JoinType.FULL, joinClause("3."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinSimpleRightLeaning()
+ {
+ // Join of a table onto a variety of simple joinable objects (lookup, inline, subquery) with a right-leaning
+ // structure (no left children are joins themselves).
+ //
+ // Note that unlike the left-leaning stack, which is fully flattened, this one will not get flattened at all.
+
+ final JoinDataSource rightLeaningJoinStack =
+ join(
+ LOOKUP_LOOKYLOO,
+ join(
+ INLINE,
+ subquery(LOOKUP_LOOKYLOO),
+ "1.",
+ JoinType.LEFT
+ ),
+ "2.",
+ JoinType.FULL
+ );
+
+ final JoinDataSource joinDataSource =
+ join(
+ TABLE_FOO,
+ rightLeaningJoinStack,
+ "3.",
+ JoinType.RIGHT
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("3.", rightLeaningJoinStack, JoinType.RIGHT, joinClause("3."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinOverTableSubquery()
+ {
+ final JoinDataSource joinDataSource = join(
+ TABLE_FOO,
+ subquery(TABLE_FOO),
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", subquery(TABLE_FOO), JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinTableUnionToLookup()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final JoinDataSource joinDataSource = join(
+ unionDataSource,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinUnderTopLevelSubqueries()
+ {
+ final QueryDataSource queryDataSource =
+ subquery(
+ subquery(
+ join(
+ TABLE_FOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ )
+ )
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinLookupToLookup()
+ {
+ final JoinDataSource joinDataSource = join(
+ LOOKUP_LOOKYLOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinLookupToTable()
+ {
+ final JoinDataSource joinDataSource = join(
+ LOOKUP_LOOKYLOO,
+ TABLE_FOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", TABLE_FOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
Review comment:
Do you want to add an `EqualsVerifier` test?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369715879
##########
File path: processing/src/main/java/org/apache/druid/query/BaseQuery.java
##########
@@ -270,14 +270,13 @@ public boolean equals(Object o)
Objects.equals(dataSource, baseQuery.dataSource) &&
Objects.equals(context, baseQuery.context) &&
Objects.equals(querySegmentSpec, baseQuery.querySegmentSpec) &&
- Objects.equals(duration, baseQuery.duration) &&
+ Objects.equals(getDuration(), baseQuery.getDuration()) &&
Objects.equals(granularity, baseQuery.granularity);
}
@Override
public int hashCode()
{
-
- return Objects.hash(dataSource, descending, context, querySegmentSpec, duration, granularity);
Review comment:
Weird - I see the calls to `super.equals()` now, maybe I just shouldn't read code in the night 😝
EqualsVerifier is still complaining about some other stuff. But cleaning that up can be another issue.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369336798
##########
File path: processing/src/main/java/org/apache/druid/query/InlineDataSource.java
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.RowAdapter;
+import org.apache.druid.segment.column.ColumnHolder;
+import org.apache.druid.segment.column.ValueType;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.ToLongFunction;
+
+/**
+ * Represents an inline datasource, where the rows are embedded within the DataSource object itself.
+ *
+ * The rows are backed by an Iterable, which can be lazy or not. Lazy datasources will only be iterated if someone calls
+ * {@link #getRows()} and iterates the result, or until someone calls {@link #getRowsAsList()}.
+ */
+public class InlineDataSource implements DataSource
+{
+ private final List<String> columnNames;
+ private final List<ValueType> columnTypes;
+ private final Iterable<Object[]> rows;
+
+ private InlineDataSource(
+ final List<String> columnNames,
+ final List<ValueType> columnTypes,
+ final Iterable<Object[]> rows
+ )
+ {
+ this.columnNames = Preconditions.checkNotNull(columnNames, "'columnNames' must be nonnull");
+ this.columnTypes = Preconditions.checkNotNull(columnTypes, "'columnTypes' must be nonnull");
+ this.rows = Preconditions.checkNotNull(rows, "'rows' must be nonnull");
+
+ if (columnNames.size() != columnTypes.size()) {
+ throw new IAE("columnNames and columnTypes must be the same length");
+ }
+ }
+
+ /**
+ * Factory method for Jackson. Used for inline datasources that were originally encoded as JSON. Private because
+ * non-Jackson callers should use {@link #fromIterable}.
+ */
+ @JsonCreator
+ public static InlineDataSource fromJson(
Review comment:
Oops, yes, it should. I'll fix it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369701195
##########
File path: processing/src/main/java/org/apache/druid/query/BaseQuery.java
##########
@@ -270,14 +270,13 @@ public boolean equals(Object o)
Objects.equals(dataSource, baseQuery.dataSource) &&
Objects.equals(context, baseQuery.context) &&
Objects.equals(querySegmentSpec, baseQuery.querySegmentSpec) &&
- Objects.equals(duration, baseQuery.duration) &&
+ Objects.equals(getDuration(), baseQuery.getDuration()) &&
Objects.equals(granularity, baseQuery.granularity);
}
@Override
public int hashCode()
{
-
- return Objects.hash(dataSource, descending, context, querySegmentSpec, duration, granularity);
Review comment:
It's because `duration` is lazily computed, so it might be null. Calling `getDuration()` forces it to be computed. I'll add a comment.
Btw, a lot of the subclasses of BaseQuery _do_ call `super.equals` at some point (but they do other stuff too).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369347559
##########
File path: server/src/main/java/org/apache/druid/client/CacheUtil.java
##########
@@ -99,7 +99,8 @@
return QueryContexts.isUseCache(query)
&& strategy != null
&& cacheConfig.isUseCache()
- && cacheConfig.isQueryCacheable(query);
+ && cacheConfig.isQueryCacheable(query)
+ && query.getDataSource().isCacheable();
Review comment:
Hmm, after thinking about this a bit IMO it is best to combine ResultLevelCacheUtil and CacheUtil, then make the change you suggested (although I think we can only extract three lines), and then add tests for CacheUtil. I'll add that to this patch in a bit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369339228
##########
File path: processing/src/main/java/org/apache/druid/segment/join/Joinables.java
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.join;
+
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.column.ColumnHolder;
+
+import javax.annotation.Nullable;
+
+/**
+ * Utility methods for working with {@link Joinable} related classes.
+ */
+public class Joinables
Review comment:
Good idea, I'll add some.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369290698
##########
File path: server/src/main/java/org/apache/druid/client/CacheUtil.java
##########
@@ -99,7 +99,8 @@
return QueryContexts.isUseCache(query)
&& strategy != null
&& cacheConfig.isUseCache()
- && cacheConfig.isQueryCacheable(query);
+ && cacheConfig.isQueryCacheable(query)
+ && query.getDataSource().isCacheable();
Review comment:
Perhaps extract these four lines should be a helper method since it's duplicated in populateResultLevelCache().
Is there a test that is affected by the addition of `isCacheable()` or should one be added?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369423748
##########
File path: processing/src/main/java/org/apache/druid/query/UnionDataSource.java
##########
@@ -52,6 +57,51 @@ public UnionDataSource(@JsonProperty("dataSources") List<TableDataSource> dataSo
return dataSources;
}
+ @Override
+ public List<DataSource> getChildren()
+ {
+ return ImmutableList.copyOf(dataSources);
+ }
+
+ @Override
+ public DataSource withChildren(List<DataSource> children)
+ {
+ if (children.size() != dataSources.size()) {
Review comment:
why must these sizes match?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369313963
##########
File path: processing/src/main/java/org/apache/druid/segment/join/Joinables.java
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.join;
+
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.column.ColumnHolder;
+
+import javax.annotation.Nullable;
+
+/**
+ * Utility methods for working with {@link Joinable} related classes.
+ */
+public class Joinables
Review comment:
Do you want to add unit tests for these methods?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369700275
##########
File path: processing/src/main/java/org/apache/druid/query/UnionDataSource.java
##########
@@ -52,6 +57,51 @@ public UnionDataSource(@JsonProperty("dataSources") List<TableDataSource> dataSo
return dataSources;
}
+ @Override
+ public List<DataSource> getChildren()
+ {
+ return ImmutableList.copyOf(dataSources);
+ }
+
+ @Override
+ public DataSource withChildren(List<DataSource> children)
+ {
+ if (children.size() != dataSources.size()) {
Review comment:
It's a precondition for the method, since its intent is for replacing inputs with inlined/etc versions, so the number of children shouldn't change. I'll add a comment in the interface.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369699154
##########
File path: server/src/main/java/org/apache/druid/client/CacheUtil.java
##########
@@ -99,7 +99,8 @@
return QueryContexts.isUseCache(query)
&& strategy != null
&& cacheConfig.isUseCache()
- && cacheConfig.isQueryCacheable(query);
+ && cacheConfig.isQueryCacheable(query)
+ && query.getDataSource().isCacheable();
Review comment:
@ccaominh I decided to combine them after all, and add a CacheUtil test that tests the newly extracted `isQueryCacheable` method.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369325149
##########
File path: processing/src/test/java/org/apache/druid/query/planning/DataSourceAnalysisTest.java
##########
@@ -0,0 +1,476 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.planning;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.query.DataSource;
+import org.apache.druid.query.InlineDataSource;
+import org.apache.druid.query.JoinDataSource;
+import org.apache.druid.query.LookupDataSource;
+import org.apache.druid.query.QueryDataSource;
+import org.apache.druid.query.TableDataSource;
+import org.apache.druid.query.UnionDataSource;
+import org.apache.druid.query.groupby.GroupByQuery;
+import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
+import org.apache.druid.segment.column.ValueType;
+import org.apache.druid.segment.join.JoinConditionAnalysis;
+import org.apache.druid.segment.join.JoinType;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Optional;
+
+public class DataSourceAnalysisTest
+{
+ private static final TableDataSource TABLE_FOO = new TableDataSource("foo");
+ private static final TableDataSource TABLE_BAR = new TableDataSource("bar");
+ private static final LookupDataSource LOOKUP_LOOKYLOO = new LookupDataSource("lookyloo");
+ private static final InlineDataSource INLINE = InlineDataSource.fromIterable(
+ ImmutableList.of("column"),
+ ImmutableList.of(ValueType.STRING),
+ ImmutableList.of(new Object[0])
+ );
+
+ @Test
+ public void testTable()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(TABLE_FOO);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(TABLE_FOO, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(unionDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(unionDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnTable()
+ {
+ final QueryDataSource queryDataSource = subquery(TABLE_FOO);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final QueryDataSource queryDataSource = subquery(unionDataSource);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testLookup()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(LOOKUP_LOOKYLOO);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnLookup()
+ {
+ final QueryDataSource queryDataSource = new QueryDataSource(
+ GroupByQuery.builder()
+ .setDataSource(LOOKUP_LOOKYLOO)
+ .setInterval(new MultipleIntervalSegmentSpec(Collections.singletonList(Intervals.of("2000/3000"))))
+ .setGranularity(Granularities.ALL)
+ .build()
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testInline()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(INLINE);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(INLINE, analysis.getDataSource());
+ Assert.assertEquals(INLINE, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testJoinSimpleLeftLeaning()
+ {
+ // Join of a table onto a variety of simple joinable objects (lookup, inline, subquery) with a left-leaning
+ // structure (no right children are joins themselves).
+
+ final JoinDataSource joinDataSource =
+ join(
+ join(
+ join(
+ TABLE_FOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ ),
+ INLINE,
+ "2.",
+ JoinType.LEFT
+ ),
+ subquery(LOOKUP_LOOKYLOO),
+ "3.",
+ JoinType.FULL
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1.")),
+ new PreJoinableClause("2.", INLINE, JoinType.LEFT, joinClause("2.")),
+ new PreJoinableClause("3.", subquery(LOOKUP_LOOKYLOO), JoinType.FULL, joinClause("3."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinSimpleRightLeaning()
+ {
+ // Join of a table onto a variety of simple joinable objects (lookup, inline, subquery) with a right-leaning
+ // structure (no left children are joins themselves).
+ //
+ // Note that unlike the left-leaning stack, which is fully flattened, this one will not get flattened at all.
+
+ final JoinDataSource rightLeaningJoinStack =
+ join(
+ LOOKUP_LOOKYLOO,
+ join(
+ INLINE,
+ subquery(LOOKUP_LOOKYLOO),
+ "1.",
+ JoinType.LEFT
+ ),
+ "2.",
+ JoinType.FULL
+ );
+
+ final JoinDataSource joinDataSource =
+ join(
+ TABLE_FOO,
+ rightLeaningJoinStack,
+ "3.",
+ JoinType.RIGHT
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("3.", rightLeaningJoinStack, JoinType.RIGHT, joinClause("3."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinOverTableSubquery()
+ {
+ final JoinDataSource joinDataSource = join(
+ TABLE_FOO,
+ subquery(TABLE_FOO),
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", subquery(TABLE_FOO), JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinTableUnionToLookup()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final JoinDataSource joinDataSource = join(
+ unionDataSource,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinUnderTopLevelSubqueries()
+ {
+ final QueryDataSource queryDataSource =
+ subquery(
+ subquery(
+ join(
+ TABLE_FOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ )
+ )
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinLookupToLookup()
+ {
+ final JoinDataSource joinDataSource = join(
+ LOOKUP_LOOKYLOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinLookupToTable()
+ {
+ final JoinDataSource joinDataSource = join(
+ LOOKUP_LOOKYLOO,
+ TABLE_FOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", TABLE_FOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ /**
+ * Generate a datasource that joins on a column named "x" on both sides.
+ */
+ private static JoinDataSource join(
+ final DataSource left,
+ final DataSource right,
+ final String rightPrefix,
+ final JoinType joinType
+ )
+ {
+ return JoinDataSource.create(
+ left,
+ right,
+ rightPrefix,
+ joinClause(rightPrefix).getOriginalExpression(),
+ joinType,
+ ExprMacroTable.nil()
+ );
+ }
+
+ /**
+ * Generate a join clause that joins on a column named "x" on both sides.
+ */
+ private static JoinConditionAnalysis joinClause(
+ final String rightPrefix
+ )
+ {
+ return JoinConditionAnalysis.forExpression(
+ StringUtils.format("x == \"%sx\"", rightPrefix),
+ rightPrefix,
+ ExprMacroTable.nil()
+ );
+ }
+
+ /**
+ * Generate a datasource that does a subquery on another datasource. The specific kind of query doesn't matter
+ * much for the purpose of this test class, so it's always the same.
+ */
+ private static QueryDataSource subquery(final DataSource dataSource)
+ {
+ return new QueryDataSource(
+ GroupByQuery.builder()
+ .setDataSource(dataSource)
+ .setInterval(new MultipleIntervalSegmentSpec(Collections.singletonList(Intervals.of("2000/3000"))))
Review comment:
Perhaps create a named constant for the interval since it's in the asserts for several of the new tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369337345
##########
File path: processing/src/main/java/org/apache/druid/query/InlineDataSource.java
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.RowAdapter;
+import org.apache.druid.segment.column.ColumnHolder;
+import org.apache.druid.segment.column.ValueType;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.ToLongFunction;
+
+/**
+ * Represents an inline datasource, where the rows are embedded within the DataSource object itself.
+ *
+ * The rows are backed by an Iterable, which can be lazy or not. Lazy datasources will only be iterated if someone calls
+ * {@link #getRows()} and iterates the result, or until someone calls {@link #getRowsAsList()}.
+ */
+public class InlineDataSource implements DataSource
+{
+ private final List<String> columnNames;
+ private final List<ValueType> columnTypes;
+ private final Iterable<Object[]> rows;
+
+ private InlineDataSource(
+ final List<String> columnNames,
+ final List<ValueType> columnTypes,
+ final Iterable<Object[]> rows
+ )
+ {
+ this.columnNames = Preconditions.checkNotNull(columnNames, "'columnNames' must be nonnull");
+ this.columnTypes = Preconditions.checkNotNull(columnTypes, "'columnTypes' must be nonnull");
+ this.rows = Preconditions.checkNotNull(rows, "'rows' must be nonnull");
+
+ if (columnNames.size() != columnTypes.size()) {
+ throw new IAE("columnNames and columnTypes must be the same length");
+ }
+ }
+
+ /**
+ * Factory method for Jackson. Used for inline datasources that were originally encoded as JSON. Private because
+ * non-Jackson callers should use {@link #fromIterable}.
+ */
+ @JsonCreator
+ public static InlineDataSource fromJson(
+ @JsonProperty("columnNames") List<String> columnNames,
+ @JsonProperty("columnTypes") List<ValueType> columnTypes,
+ @JsonProperty("rows") List<Object[]> rows
+ )
+ {
+ return new InlineDataSource(columnNames, columnTypes, rows);
+ }
+
+ /**
+ * Creates an inline datasource from an Iterable. The Iterable will not be iterated until someone calls
+ * {@link #getRows()} and iterates the result, or until someone calls {@link #getRowsAsList()}.
+ *
+ * @param columnNames names of each column in the rows
+ * @param columnTypes types of each column in the rows
+ * @param rows rows, each of the same length as columnNames and columnTypes
+ */
+ public static InlineDataSource fromIterable(
+ final List<String> columnNames,
+ final List<ValueType> columnTypes,
+ final Iterable<Object[]> rows
+ )
+ {
+ return new InlineDataSource(columnNames, columnTypes, rows);
+ }
+
+ @Override
+ public Set<String> getTableNames()
+ {
+ return Collections.emptySet();
+ }
+
+ @JsonProperty
+ public List<String> getColumnNames()
+ {
+ return columnNames;
+ }
+
+ @JsonProperty
+ public List<ValueType> getColumnTypes()
+ {
+ return columnTypes;
+ }
+
+ /**
+ * Returns rows as a list. If the original Iterable behind this datasource was a List, this method will return it
+ * as-is, without copying it. Otherwise, this method will walk the iterable and copy it into a List before returning.
+ */
+ @JsonProperty("rows")
+ public List<Object[]> getRowsAsList()
+ {
+ return rows instanceof List ? ((List<Object[]>) rows) : Lists.newArrayList(rows);
+ }
+
+ /**
+ * Returns rows as an Iterable.
+ */
+ @JsonIgnore
+ public Iterable<Object[]> getRows()
+ {
+ return rows;
+ }
+
+ @Override
+ public List<DataSource> getChildren()
+ {
+ return Collections.emptyList();
+ }
+
+ @Override
+ public DataSource withChildren(List<DataSource> children)
+ {
+ if (!children.isEmpty()) {
+ throw new IAE("Cannot accept children");
+ }
+
+ return this;
+ }
+
+ @Override
+ public boolean isCacheable()
+ {
+ return false;
+ }
+
+ @Override
+ public boolean isGlobal()
+ {
+ return true;
+ }
+
+ @Override
+ public boolean isConcrete()
+ {
+ return false;
+ }
+
+ public Map<String, ValueType> getRowSignature()
+ {
+ final ImmutableMap.Builder<String, ValueType> retVal = ImmutableMap.builder();
+
+ for (int i = 0; i < columnNames.size(); i++) {
+ retVal.put(columnNames.get(i), columnTypes.get(i));
+ }
+
+ return retVal.build();
+ }
+
+ public RowAdapter<Object[]> rowAdapter()
+ {
+ return new RowAdapter<Object[]>()
+ {
+ @Override
+ public ToLongFunction<Object[]> timestampFunction()
+ {
+ final int columnNumber = columnNames.indexOf(ColumnHolder.TIME_COLUMN_NAME);
+
+ if (columnNumber >= 0) {
+ return row -> (long) row[columnNumber];
+ } else {
+ return row -> 0L;
+ }
+ }
+
+ @Override
+ public Function<Object[], Object> columnFunction(String columnName)
+ {
+ final int columnNumber = columnNames.indexOf(columnName);
+
+ if (columnNumber >= 0) {
+ return row -> row[columnNumber];
+ } else {
+ return row -> null;
+ }
+ }
+ };
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ InlineDataSource that = (InlineDataSource) o;
+ return Objects.equals(columnNames, that.columnNames) &&
+ Objects.equals(columnTypes, that.columnTypes) &&
+ Objects.equals(rows, that.rows);
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(columnNames, columnTypes, rows);
+ }
+
+ @Override
+ public String toString()
+ {
+ // Don't include 'rows' in stringificatione, because it might be long and/or lazy.
Review comment:
Good idea, I'll add one (and fix the typo).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369345996
##########
File path: processing/src/main/java/org/apache/druid/query/planning/DataSourceAnalysis.java
##########
@@ -0,0 +1,282 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.planning;
+
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.java.util.common.Pair;
+import org.apache.druid.query.BaseQuery;
+import org.apache.druid.query.DataSource;
+import org.apache.druid.query.JoinDataSource;
+import org.apache.druid.query.Query;
+import org.apache.druid.query.QueryDataSource;
+import org.apache.druid.query.TableDataSource;
+import org.apache.druid.query.UnionDataSource;
+import org.apache.druid.query.spec.QuerySegmentSpec;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * Analysis of a datasource for purposes of deciding how to execute a particular query.
+ *
+ * The analysis breaks a datasource down in the following way:
+ *
+ * <pre>
+ *
+ * Q <-- Possible outer query datasource(s) [may be multiple stacked]
+ * |
+ * J <-- Possible join tree, expected to be left-leaning
+ * / \
+ * J Dj <-- Other leaf datasources
+ * Base datasource / \ which will be joined
+ * (bottom-leftmost) --> Db Dj <---- into the base datasource
+ *
+ * </pre>
+ *
+ * The base datasource (Db) is returned by {@link #getBaseDataSource()}. The other leaf datasources are returned by
+ * {@link #getPreJoinableClauses()}. The outer query datasources are available as part of {@link #getDataSource()},
+ * which just returns the original datasource that was provided for analysis.
+ *
+ * The base datasource (Db) will never be a join, but it can be any other type of datasource (table, query, etc).
+ * Note that join trees are only flattened if they occur at the top of the overall tree (or underneath an outer query),
+ * and that join trees are only flattened to the degree that they are left-leaning. Due to these facts, it is possible
+ * for the base or leaf datasources to include additional joins.
+ *
+ * The base datasource is the one that will be considered by the core Druid query stack for scanning via
+ * {@link org.apache.druid.segment.Segment} and {@link org.apache.druid.segment.StorageAdapter}. The other leaf
+ * datasources must be joinable onto the base data.
+ *
+ * The idea here is to keep things simple and dumb. So we focus only on identifying left-leaning join trees, which map
+ * neatly onto a series of hash table lookups at query time. The user/system generating the queries, e.g. the druid-sql
+ * layer (or the end user in the case of native queries), is responsible for containing the smarts to structure the
+ * tree in a way that will lead to optimal execution.
+ */
Review comment:
ASCII art is what it's all about.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #9235:
Add join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369259903
##########
File path: processing/src/main/java/org/apache/druid/query/InlineDataSource.java
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.RowAdapter;
+import org.apache.druid.segment.column.ColumnHolder;
+import org.apache.druid.segment.column.ValueType;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.ToLongFunction;
+
+/**
+ * Represents an inline datasource, where the rows are embedded within the DataSource object itself.
+ *
+ * The rows are backed by an Iterable, which can be lazy or not. Lazy datasources will only be iterated if someone calls
+ * {@link #getRows()} and iterates the result, or until someone calls {@link #getRowsAsList()}.
+ */
+public class InlineDataSource implements DataSource
Review comment:
:metal:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369323251
##########
File path: processing/src/test/java/org/apache/druid/query/planning/DataSourceAnalysisTest.java
##########
@@ -0,0 +1,476 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.planning;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.query.DataSource;
+import org.apache.druid.query.InlineDataSource;
+import org.apache.druid.query.JoinDataSource;
+import org.apache.druid.query.LookupDataSource;
+import org.apache.druid.query.QueryDataSource;
+import org.apache.druid.query.TableDataSource;
+import org.apache.druid.query.UnionDataSource;
+import org.apache.druid.query.groupby.GroupByQuery;
+import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
+import org.apache.druid.segment.column.ValueType;
+import org.apache.druid.segment.join.JoinConditionAnalysis;
+import org.apache.druid.segment.join.JoinType;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Optional;
+
+public class DataSourceAnalysisTest
+{
+ private static final TableDataSource TABLE_FOO = new TableDataSource("foo");
+ private static final TableDataSource TABLE_BAR = new TableDataSource("bar");
+ private static final LookupDataSource LOOKUP_LOOKYLOO = new LookupDataSource("lookyloo");
+ private static final InlineDataSource INLINE = InlineDataSource.fromIterable(
+ ImmutableList.of("column"),
+ ImmutableList.of(ValueType.STRING),
+ ImmutableList.of(new Object[0])
+ );
+
+ @Test
+ public void testTable()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(TABLE_FOO);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(TABLE_FOO, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(unionDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(unionDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnTable()
+ {
+ final QueryDataSource queryDataSource = subquery(TABLE_FOO);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final QueryDataSource queryDataSource = subquery(unionDataSource);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testLookup()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(LOOKUP_LOOKYLOO);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnLookup()
+ {
+ final QueryDataSource queryDataSource = new QueryDataSource(
+ GroupByQuery.builder()
+ .setDataSource(LOOKUP_LOOKYLOO)
+ .setInterval(new MultipleIntervalSegmentSpec(Collections.singletonList(Intervals.of("2000/3000"))))
+ .setGranularity(Granularities.ALL)
+ .build()
+ );
Review comment:
Looks like you can use `subquery(LOOKUP_LOOKYLOO)` here instead?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #9235:
Add join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369254935
##########
File path: processing/src/main/java/org/apache/druid/query/UnionDataSource.java
##########
@@ -52,6 +57,51 @@ public UnionDataSource(@JsonProperty("dataSources") List<TableDataSource> dataSo
return dataSources;
}
+ @Override
+ public List<DataSource> getChildren()
+ {
+ return ImmutableList.copyOf(dataSources);
+ }
+
+ @Override
+ public DataSource withChildren(List<DataSource> children)
+ {
+ if (children.size() != dataSources.size()) {
+ throw new IAE("Expected [%d] children, got [%d]", dataSources.size(), children.size());
+ }
+
+ if (!children.stream().allMatch(dataSource -> dataSource instanceof TableDataSource)) {
+ throw new IAE("All children must be tables");
+ }
+
+ return new UnionDataSource(
+ children.stream().map(dataSource -> (TableDataSource) dataSource).collect(Collectors.toList())
+ );
+ }
+
+ @Override
+ public boolean isCacheable()
+ {
+ // Disables result-level caching for 'union' datasources, which doesn't work currently.
+ // See https://github.com/apache/druid/issues/8713 for reference.
Review comment:
:+1: this seems like a good intermediary solution until we fix the issue for real
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369317286
##########
File path: processing/src/main/java/org/apache/druid/query/planning/DataSourceAnalysis.java
##########
@@ -0,0 +1,282 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.planning;
+
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.java.util.common.Pair;
+import org.apache.druid.query.BaseQuery;
+import org.apache.druid.query.DataSource;
+import org.apache.druid.query.JoinDataSource;
+import org.apache.druid.query.Query;
+import org.apache.druid.query.QueryDataSource;
+import org.apache.druid.query.TableDataSource;
+import org.apache.druid.query.UnionDataSource;
+import org.apache.druid.query.spec.QuerySegmentSpec;
+
+import javax.annotation.Nullable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+
+/**
+ * Analysis of a datasource for purposes of deciding how to execute a particular query.
+ *
+ * The analysis breaks a datasource down in the following way:
+ *
+ * <pre>
+ *
+ * Q <-- Possible outer query datasource(s) [may be multiple stacked]
+ * |
+ * J <-- Possible join tree, expected to be left-leaning
+ * / \
+ * J Dj <-- Other leaf datasources
+ * Base datasource / \ which will be joined
+ * (bottom-leftmost) --> Db Dj <---- into the base datasource
+ *
+ * </pre>
+ *
+ * The base datasource (Db) is returned by {@link #getBaseDataSource()}. The other leaf datasources are returned by
+ * {@link #getPreJoinableClauses()}. The outer query datasources are available as part of {@link #getDataSource()},
+ * which just returns the original datasource that was provided for analysis.
+ *
+ * The base datasource (Db) will never be a join, but it can be any other type of datasource (table, query, etc).
+ * Note that join trees are only flattened if they occur at the top of the overall tree (or underneath an outer query),
+ * and that join trees are only flattened to the degree that they are left-leaning. Due to these facts, it is possible
+ * for the base or leaf datasources to include additional joins.
+ *
+ * The base datasource is the one that will be considered by the core Druid query stack for scanning via
+ * {@link org.apache.druid.segment.Segment} and {@link org.apache.druid.segment.StorageAdapter}. The other leaf
+ * datasources must be joinable onto the base data.
+ *
+ * The idea here is to keep things simple and dumb. So we focus only on identifying left-leaning join trees, which map
+ * neatly onto a series of hash table lookups at query time. The user/system generating the queries, e.g. the druid-sql
+ * layer (or the end user in the case of native queries), is responsible for containing the smarts to structure the
+ * tree in a way that will lead to optimal execution.
+ */
Review comment:
Nice javadoc!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369289411
##########
File path: server/src/main/java/org/apache/druid/client/ResultLevelCacheUtil.java
##########
@@ -73,7 +73,8 @@ public static void populate(
return QueryContexts.isUseResultLevelCache(query)
&& strategy != null
&& cacheConfig.isUseResultLevelCache()
- && cacheConfig.isQueryCacheable(query);
+ && cacheConfig.isQueryCacheable(query)
+ && query.getDataSource().isCacheable();
Review comment:
Similar comment to the one I left in `CacheUtil`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369700647
##########
File path: processing/src/main/java/org/apache/druid/segment/join/Joinables.java
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.join;
+
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.column.ColumnHolder;
+
+import javax.annotation.Nullable;
+
+/**
+ * Utility methods for working with {@link Joinable} related classes.
+ */
+public class Joinables
+{
+ /**
+ * Checks that "prefix" is a valid prefix for a join clause (see {@link JoinableClause#getPrefix()}) and, if so,
+ * returns it. Otherwise, throws an exception.
+ */
+ public static String validatePrefix(@Nullable final String prefix)
+ {
+ if (prefix == null) {
+ throw new IAE("Join clause cannot have null prefix");
+ } else if (isPrefixedBy(ColumnHolder.TIME_COLUMN_NAME, prefix) || ColumnHolder.TIME_COLUMN_NAME.equals(prefix)) {
+ throw new IAE(
+ "Join clause cannot have prefix[%s], since it would shadow %s",
+ prefix,
+ ColumnHolder.TIME_COLUMN_NAME
+ );
+ } else {
+ return prefix;
+ }
+ }
+
+ public static boolean isPrefixedBy(final String columnName, final String prefix)
Review comment:
I think I'd prefer `@EverythingIsNonnullByDefault` (which is how I prefer to write/read code usually anyway). We use it in a few other packages in Druid. Maybe we can add that here in a future patch?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369345857
##########
File path: processing/src/test/java/org/apache/druid/query/planning/DataSourceAnalysisTest.java
##########
@@ -0,0 +1,476 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.planning;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.query.DataSource;
+import org.apache.druid.query.InlineDataSource;
+import org.apache.druid.query.JoinDataSource;
+import org.apache.druid.query.LookupDataSource;
+import org.apache.druid.query.QueryDataSource;
+import org.apache.druid.query.TableDataSource;
+import org.apache.druid.query.UnionDataSource;
+import org.apache.druid.query.groupby.GroupByQuery;
+import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
+import org.apache.druid.segment.column.ValueType;
+import org.apache.druid.segment.join.JoinConditionAnalysis;
+import org.apache.druid.segment.join.JoinType;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Optional;
+
+public class DataSourceAnalysisTest
+{
+ private static final TableDataSource TABLE_FOO = new TableDataSource("foo");
+ private static final TableDataSource TABLE_BAR = new TableDataSource("bar");
+ private static final LookupDataSource LOOKUP_LOOKYLOO = new LookupDataSource("lookyloo");
+ private static final InlineDataSource INLINE = InlineDataSource.fromIterable(
+ ImmutableList.of("column"),
+ ImmutableList.of(ValueType.STRING),
+ ImmutableList.of(new Object[0])
+ );
+
+ @Test
+ public void testTable()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(TABLE_FOO);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(TABLE_FOO, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(unionDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(unionDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnTable()
+ {
+ final QueryDataSource queryDataSource = subquery(TABLE_FOO);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final QueryDataSource queryDataSource = subquery(unionDataSource);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testLookup()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(LOOKUP_LOOKYLOO);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnLookup()
+ {
+ final QueryDataSource queryDataSource = new QueryDataSource(
+ GroupByQuery.builder()
+ .setDataSource(LOOKUP_LOOKYLOO)
+ .setInterval(new MultipleIntervalSegmentSpec(Collections.singletonList(Intervals.of("2000/3000"))))
+ .setGranularity(Granularities.ALL)
+ .build()
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testInline()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(INLINE);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(INLINE, analysis.getDataSource());
+ Assert.assertEquals(INLINE, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testJoinSimpleLeftLeaning()
+ {
+ // Join of a table onto a variety of simple joinable objects (lookup, inline, subquery) with a left-leaning
+ // structure (no right children are joins themselves).
+
+ final JoinDataSource joinDataSource =
+ join(
+ join(
+ join(
+ TABLE_FOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ ),
+ INLINE,
+ "2.",
+ JoinType.LEFT
+ ),
+ subquery(LOOKUP_LOOKYLOO),
+ "3.",
+ JoinType.FULL
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1.")),
+ new PreJoinableClause("2.", INLINE, JoinType.LEFT, joinClause("2.")),
+ new PreJoinableClause("3.", subquery(LOOKUP_LOOKYLOO), JoinType.FULL, joinClause("3."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinSimpleRightLeaning()
+ {
+ // Join of a table onto a variety of simple joinable objects (lookup, inline, subquery) with a right-leaning
+ // structure (no left children are joins themselves).
+ //
+ // Note that unlike the left-leaning stack, which is fully flattened, this one will not get flattened at all.
+
+ final JoinDataSource rightLeaningJoinStack =
+ join(
+ LOOKUP_LOOKYLOO,
+ join(
+ INLINE,
+ subquery(LOOKUP_LOOKYLOO),
+ "1.",
+ JoinType.LEFT
+ ),
+ "2.",
+ JoinType.FULL
+ );
+
+ final JoinDataSource joinDataSource =
+ join(
+ TABLE_FOO,
+ rightLeaningJoinStack,
+ "3.",
+ JoinType.RIGHT
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("3.", rightLeaningJoinStack, JoinType.RIGHT, joinClause("3."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinOverTableSubquery()
+ {
+ final JoinDataSource joinDataSource = join(
+ TABLE_FOO,
+ subquery(TABLE_FOO),
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", subquery(TABLE_FOO), JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinTableUnionToLookup()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final JoinDataSource joinDataSource = join(
+ unionDataSource,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinUnderTopLevelSubqueries()
+ {
+ final QueryDataSource queryDataSource =
+ subquery(
+ subquery(
+ join(
+ TABLE_FOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ )
+ )
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinLookupToLookup()
+ {
+ final JoinDataSource joinDataSource = join(
+ LOOKUP_LOOKYLOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinLookupToTable()
+ {
+ final JoinDataSource joinDataSource = join(
+ LOOKUP_LOOKYLOO,
+ TABLE_FOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", TABLE_FOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ /**
+ * Generate a datasource that joins on a column named "x" on both sides.
+ */
+ private static JoinDataSource join(
+ final DataSource left,
+ final DataSource right,
+ final String rightPrefix,
+ final JoinType joinType
+ )
+ {
+ return JoinDataSource.create(
+ left,
+ right,
+ rightPrefix,
+ joinClause(rightPrefix).getOriginalExpression(),
+ joinType,
+ ExprMacroTable.nil()
+ );
+ }
+
+ /**
+ * Generate a join clause that joins on a column named "x" on both sides.
+ */
+ private static JoinConditionAnalysis joinClause(
+ final String rightPrefix
+ )
+ {
+ return JoinConditionAnalysis.forExpression(
+ StringUtils.format("x == \"%sx\"", rightPrefix),
+ rightPrefix,
+ ExprMacroTable.nil()
+ );
+ }
+
+ /**
+ * Generate a datasource that does a subquery on another datasource. The specific kind of query doesn't matter
+ * much for the purpose of this test class, so it's always the same.
+ */
+ private static QueryDataSource subquery(final DataSource dataSource)
+ {
+ return new QueryDataSource(
+ GroupByQuery.builder()
+ .setDataSource(dataSource)
+ .setInterval(new MultipleIntervalSegmentSpec(Collections.singletonList(Intervals.of("2000/3000"))))
Review comment:
Sure, sounds reasonable.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm merged pull request #9235: Add join-related
DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm merged pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369850349
##########
File path: processing/src/test/java/org/apache/druid/query/InlineDataSourceTest.java
##########
@@ -225,6 +225,14 @@ public void test_equals()
.verify();
}
+ @Test
+ public void test_toString_iterable()
+ {
+ // Verify that toString does not iterate the rows.
+ final String ignored = iterableDataSource.toString();
+ Assert.assertEquals(0, iterationCounter.get());
Review comment:
Any suggestions for how to improve it in a follow-up?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369849595
##########
File path: processing/src/test/java/org/apache/druid/query/InlineDataSourceTest.java
##########
@@ -225,6 +225,14 @@ public void test_equals()
.verify();
}
+ @Test
+ public void test_toString_iterable()
+ {
+ // Verify that toString does not iterate the rows.
+ final String ignored = iterableDataSource.toString();
+ Assert.assertEquals(0, iterationCounter.get());
Review comment:
I'm ok with leaving the test as it is for now.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369428114
##########
File path: processing/src/main/java/org/apache/druid/segment/join/Joinables.java
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.join;
+
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.column.ColumnHolder;
+
+import javax.annotation.Nullable;
+
+/**
+ * Utility methods for working with {@link Joinable} related classes.
+ */
+public class Joinables
+{
+ /**
+ * Checks that "prefix" is a valid prefix for a join clause (see {@link JoinableClause#getPrefix()}) and, if so,
+ * returns it. Otherwise, throws an exception.
+ */
+ public static String validatePrefix(@Nullable final String prefix)
+ {
+ if (prefix == null) {
+ throw new IAE("Join clause cannot have null prefix");
+ } else if (isPrefixedBy(ColumnHolder.TIME_COLUMN_NAME, prefix) || ColumnHolder.TIME_COLUMN_NAME.equals(prefix)) {
+ throw new IAE(
+ "Join clause cannot have prefix[%s], since it would shadow %s",
+ prefix,
+ ColumnHolder.TIME_COLUMN_NAME
+ );
+ } else {
+ return prefix;
+ }
+ }
+
+ public static boolean isPrefixedBy(final String columnName, final String prefix)
Review comment:
`@NonNull` annotations for the parameters?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369293146
##########
File path: processing/src/main/java/org/apache/druid/query/InlineDataSource.java
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.RowAdapter;
+import org.apache.druid.segment.column.ColumnHolder;
+import org.apache.druid.segment.column.ValueType;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.ToLongFunction;
+
+/**
+ * Represents an inline datasource, where the rows are embedded within the DataSource object itself.
+ *
+ * The rows are backed by an Iterable, which can be lazy or not. Lazy datasources will only be iterated if someone calls
+ * {@link #getRows()} and iterates the result, or until someone calls {@link #getRowsAsList()}.
+ */
+public class InlineDataSource implements DataSource
+{
+ private final List<String> columnNames;
+ private final List<ValueType> columnTypes;
+ private final Iterable<Object[]> rows;
+
+ private InlineDataSource(
+ final List<String> columnNames,
+ final List<ValueType> columnTypes,
+ final Iterable<Object[]> rows
+ )
+ {
+ this.columnNames = Preconditions.checkNotNull(columnNames, "'columnNames' must be nonnull");
+ this.columnTypes = Preconditions.checkNotNull(columnTypes, "'columnTypes' must be nonnull");
+ this.rows = Preconditions.checkNotNull(rows, "'rows' must be nonnull");
+
+ if (columnNames.size() != columnTypes.size()) {
+ throw new IAE("columnNames and columnTypes must be the same length");
+ }
+ }
+
+ /**
+ * Factory method for Jackson. Used for inline datasources that were originally encoded as JSON. Private because
+ * non-Jackson callers should use {@link #fromIterable}.
+ */
+ @JsonCreator
+ public static InlineDataSource fromJson(
Review comment:
Should this be `private` to match the javadoc?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on issue #9235: Add join-related DataSource
types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on issue #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#issuecomment-577298660
@ccaominh, @suneet-s — I pushed up a new commit reflecting your comments. Let me know what you think.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369339967
##########
File path: processing/src/test/java/org/apache/druid/query/planning/DataSourceAnalysisTest.java
##########
@@ -0,0 +1,476 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.planning;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.query.DataSource;
+import org.apache.druid.query.InlineDataSource;
+import org.apache.druid.query.JoinDataSource;
+import org.apache.druid.query.LookupDataSource;
+import org.apache.druid.query.QueryDataSource;
+import org.apache.druid.query.TableDataSource;
+import org.apache.druid.query.UnionDataSource;
+import org.apache.druid.query.groupby.GroupByQuery;
+import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
+import org.apache.druid.segment.column.ValueType;
+import org.apache.druid.segment.join.JoinConditionAnalysis;
+import org.apache.druid.segment.join.JoinType;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Optional;
+
+public class DataSourceAnalysisTest
+{
+ private static final TableDataSource TABLE_FOO = new TableDataSource("foo");
+ private static final TableDataSource TABLE_BAR = new TableDataSource("bar");
+ private static final LookupDataSource LOOKUP_LOOKYLOO = new LookupDataSource("lookyloo");
+ private static final InlineDataSource INLINE = InlineDataSource.fromIterable(
+ ImmutableList.of("column"),
+ ImmutableList.of(ValueType.STRING),
+ ImmutableList.of(new Object[0])
+ );
+
+ @Test
+ public void testTable()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(TABLE_FOO);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(TABLE_FOO, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(unionDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(unionDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnTable()
+ {
+ final QueryDataSource queryDataSource = subquery(TABLE_FOO);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final QueryDataSource queryDataSource = subquery(unionDataSource);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testLookup()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(LOOKUP_LOOKYLOO);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnLookup()
+ {
+ final QueryDataSource queryDataSource = new QueryDataSource(
+ GroupByQuery.builder()
+ .setDataSource(LOOKUP_LOOKYLOO)
+ .setInterval(new MultipleIntervalSegmentSpec(Collections.singletonList(Intervals.of("2000/3000"))))
+ .setGranularity(Granularities.ALL)
+ .build()
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testInline()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(INLINE);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(INLINE, analysis.getDataSource());
+ Assert.assertEquals(INLINE, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testJoinSimpleLeftLeaning()
+ {
+ // Join of a table onto a variety of simple joinable objects (lookup, inline, subquery) with a left-leaning
+ // structure (no right children are joins themselves).
+
+ final JoinDataSource joinDataSource =
+ join(
+ join(
+ join(
+ TABLE_FOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ ),
+ INLINE,
+ "2.",
+ JoinType.LEFT
+ ),
+ subquery(LOOKUP_LOOKYLOO),
+ "3.",
+ JoinType.FULL
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1.")),
+ new PreJoinableClause("2.", INLINE, JoinType.LEFT, joinClause("2.")),
+ new PreJoinableClause("3.", subquery(LOOKUP_LOOKYLOO), JoinType.FULL, joinClause("3."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinSimpleRightLeaning()
+ {
+ // Join of a table onto a variety of simple joinable objects (lookup, inline, subquery) with a right-leaning
+ // structure (no left children are joins themselves).
+ //
+ // Note that unlike the left-leaning stack, which is fully flattened, this one will not get flattened at all.
+
+ final JoinDataSource rightLeaningJoinStack =
+ join(
+ LOOKUP_LOOKYLOO,
+ join(
+ INLINE,
+ subquery(LOOKUP_LOOKYLOO),
+ "1.",
+ JoinType.LEFT
+ ),
+ "2.",
+ JoinType.FULL
+ );
+
+ final JoinDataSource joinDataSource =
+ join(
+ TABLE_FOO,
+ rightLeaningJoinStack,
+ "3.",
+ JoinType.RIGHT
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("3.", rightLeaningJoinStack, JoinType.RIGHT, joinClause("3."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinOverTableSubquery()
+ {
+ final JoinDataSource joinDataSource = join(
+ TABLE_FOO,
+ subquery(TABLE_FOO),
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", subquery(TABLE_FOO), JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinTableUnionToLookup()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final JoinDataSource joinDataSource = join(
+ unionDataSource,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinUnderTopLevelSubqueries()
+ {
+ final QueryDataSource queryDataSource =
+ subquery(
+ subquery(
+ join(
+ TABLE_FOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ )
+ )
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinLookupToLookup()
+ {
+ final JoinDataSource joinDataSource = join(
+ LOOKUP_LOOKYLOO,
+ LOOKUP_LOOKYLOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", LOOKUP_LOOKYLOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
+ @Test
+ public void testJoinLookupToTable()
+ {
+ final JoinDataSource joinDataSource = join(
+ LOOKUP_LOOKYLOO,
+ TABLE_FOO,
+ "1.",
+ JoinType.INNER
+ );
+
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(joinDataSource);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(joinDataSource, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(
+ ImmutableList.of(
+ new PreJoinableClause("1.", TABLE_FOO, JoinType.INNER, joinClause("1."))
+ ),
+ analysis.getPreJoinableClauses()
+ );
+ }
+
Review comment:
Yeah, I'll add one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369300125
##########
File path: processing/src/main/java/org/apache/druid/query/InlineDataSource.java
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.RowAdapter;
+import org.apache.druid.segment.column.ColumnHolder;
+import org.apache.druid.segment.column.ValueType;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.function.ToLongFunction;
+
+/**
+ * Represents an inline datasource, where the rows are embedded within the DataSource object itself.
+ *
+ * The rows are backed by an Iterable, which can be lazy or not. Lazy datasources will only be iterated if someone calls
+ * {@link #getRows()} and iterates the result, or until someone calls {@link #getRowsAsList()}.
+ */
+public class InlineDataSource implements DataSource
+{
+ private final List<String> columnNames;
+ private final List<ValueType> columnTypes;
+ private final Iterable<Object[]> rows;
+
+ private InlineDataSource(
+ final List<String> columnNames,
+ final List<ValueType> columnTypes,
+ final Iterable<Object[]> rows
+ )
+ {
+ this.columnNames = Preconditions.checkNotNull(columnNames, "'columnNames' must be nonnull");
+ this.columnTypes = Preconditions.checkNotNull(columnTypes, "'columnTypes' must be nonnull");
+ this.rows = Preconditions.checkNotNull(rows, "'rows' must be nonnull");
+
+ if (columnNames.size() != columnTypes.size()) {
+ throw new IAE("columnNames and columnTypes must be the same length");
+ }
+ }
+
+ /**
+ * Factory method for Jackson. Used for inline datasources that were originally encoded as JSON. Private because
+ * non-Jackson callers should use {@link #fromIterable}.
+ */
+ @JsonCreator
+ public static InlineDataSource fromJson(
+ @JsonProperty("columnNames") List<String> columnNames,
+ @JsonProperty("columnTypes") List<ValueType> columnTypes,
+ @JsonProperty("rows") List<Object[]> rows
+ )
+ {
+ return new InlineDataSource(columnNames, columnTypes, rows);
+ }
+
+ /**
+ * Creates an inline datasource from an Iterable. The Iterable will not be iterated until someone calls
+ * {@link #getRows()} and iterates the result, or until someone calls {@link #getRowsAsList()}.
+ *
+ * @param columnNames names of each column in the rows
+ * @param columnTypes types of each column in the rows
+ * @param rows rows, each of the same length as columnNames and columnTypes
+ */
+ public static InlineDataSource fromIterable(
+ final List<String> columnNames,
+ final List<ValueType> columnTypes,
+ final Iterable<Object[]> rows
+ )
+ {
+ return new InlineDataSource(columnNames, columnTypes, rows);
+ }
+
+ @Override
+ public Set<String> getTableNames()
+ {
+ return Collections.emptySet();
+ }
+
+ @JsonProperty
+ public List<String> getColumnNames()
+ {
+ return columnNames;
+ }
+
+ @JsonProperty
+ public List<ValueType> getColumnTypes()
+ {
+ return columnTypes;
+ }
+
+ /**
+ * Returns rows as a list. If the original Iterable behind this datasource was a List, this method will return it
+ * as-is, without copying it. Otherwise, this method will walk the iterable and copy it into a List before returning.
+ */
+ @JsonProperty("rows")
+ public List<Object[]> getRowsAsList()
+ {
+ return rows instanceof List ? ((List<Object[]>) rows) : Lists.newArrayList(rows);
+ }
+
+ /**
+ * Returns rows as an Iterable.
+ */
+ @JsonIgnore
+ public Iterable<Object[]> getRows()
+ {
+ return rows;
+ }
+
+ @Override
+ public List<DataSource> getChildren()
+ {
+ return Collections.emptyList();
+ }
+
+ @Override
+ public DataSource withChildren(List<DataSource> children)
+ {
+ if (!children.isEmpty()) {
+ throw new IAE("Cannot accept children");
+ }
+
+ return this;
+ }
+
+ @Override
+ public boolean isCacheable()
+ {
+ return false;
+ }
+
+ @Override
+ public boolean isGlobal()
+ {
+ return true;
+ }
+
+ @Override
+ public boolean isConcrete()
+ {
+ return false;
+ }
+
+ public Map<String, ValueType> getRowSignature()
+ {
+ final ImmutableMap.Builder<String, ValueType> retVal = ImmutableMap.builder();
+
+ for (int i = 0; i < columnNames.size(); i++) {
+ retVal.put(columnNames.get(i), columnTypes.get(i));
+ }
+
+ return retVal.build();
+ }
+
+ public RowAdapter<Object[]> rowAdapter()
+ {
+ return new RowAdapter<Object[]>()
+ {
+ @Override
+ public ToLongFunction<Object[]> timestampFunction()
+ {
+ final int columnNumber = columnNames.indexOf(ColumnHolder.TIME_COLUMN_NAME);
+
+ if (columnNumber >= 0) {
+ return row -> (long) row[columnNumber];
+ } else {
+ return row -> 0L;
+ }
+ }
+
+ @Override
+ public Function<Object[], Object> columnFunction(String columnName)
+ {
+ final int columnNumber = columnNames.indexOf(columnName);
+
+ if (columnNumber >= 0) {
+ return row -> row[columnNumber];
+ } else {
+ return row -> null;
+ }
+ }
+ };
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ InlineDataSource that = (InlineDataSource) o;
+ return Objects.equals(columnNames, that.columnNames) &&
+ Objects.equals(columnTypes, that.columnTypes) &&
+ Objects.equals(rows, that.rows);
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(columnNames, columnTypes, rows);
+ }
+
+ @Override
+ public String toString()
+ {
+ // Don't include 'rows' in stringificatione, because it might be long and/or lazy.
Review comment:
Typo: stringificatione -> stringification
Is it worth adding a test to enforce this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] ccaominh commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
ccaominh commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369817501
##########
File path: processing/src/test/java/org/apache/druid/query/InlineDataSourceTest.java
##########
@@ -225,6 +225,14 @@ public void test_equals()
.verify();
}
+ @Test
+ public void test_toString_iterable()
+ {
+ // Verify that toString does not iterate the rows.
+ final String ignored = iterableDataSource.toString();
+ Assert.assertEquals(0, iterationCounter.get());
Review comment:
I tried adding `rows` to `toString` and it doesn't iterate over `rowsIterable` (it prints an identifier for the lambda).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369410832
##########
File path: processing/src/main/java/org/apache/druid/query/BaseQuery.java
##########
@@ -270,14 +270,13 @@ public boolean equals(Object o)
Objects.equals(dataSource, baseQuery.dataSource) &&
Objects.equals(context, baseQuery.context) &&
Objects.equals(querySegmentSpec, baseQuery.querySegmentSpec) &&
- Objects.equals(duration, baseQuery.duration) &&
+ Objects.equals(getDuration(), baseQuery.getDuration()) &&
Objects.equals(granularity, baseQuery.granularity);
}
@Override
public int hashCode()
{
-
- return Objects.hash(dataSource, descending, context, querySegmentSpec, duration, granularity);
Review comment:
curious about why this change. I know this isn't really part of your change, but I've also noticed equals and hashcode for a lot of classes that extend BaseQuery have their own implementation of equals and hashCode and do not check the base class. Sounds like we should add some equalsVerifier tests for them, maybe in another patch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369832942
##########
File path: processing/src/test/java/org/apache/druid/query/InlineDataSourceTest.java
##########
@@ -225,6 +225,14 @@ public void test_equals()
.verify();
}
+ @Test
+ public void test_toString_iterable()
+ {
+ // Verify that toString does not iterate the rows.
+ final String ignored = iterableDataSource.toString();
+ Assert.assertEquals(0, iterationCounter.get());
Review comment:
Yeah, that makes sense. I guess the behavior would depend on whatever the particular Iterable’s toString method does. Do you think it makes sense to leave the test like this it change it somehow?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369709203
##########
File path: processing/src/main/java/org/apache/druid/segment/join/Joinables.java
##########
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.segment.join;
+
+import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.segment.column.ColumnHolder;
+
+import javax.annotation.Nullable;
+
+/**
+ * Utility methods for working with {@link Joinable} related classes.
+ */
+public class Joinables
+{
+ /**
+ * Checks that "prefix" is a valid prefix for a join clause (see {@link JoinableClause#getPrefix()}) and, if so,
+ * returns it. Otherwise, throws an exception.
+ */
+ public static String validatePrefix(@Nullable final String prefix)
+ {
+ if (prefix == null) {
+ throw new IAE("Join clause cannot have null prefix");
+ } else if (isPrefixedBy(ColumnHolder.TIME_COLUMN_NAME, prefix) || ColumnHolder.TIME_COLUMN_NAME.equals(prefix)) {
+ throw new IAE(
+ "Join clause cannot have prefix[%s], since it would shadow %s",
+ prefix,
+ ColumnHolder.TIME_COLUMN_NAME
+ );
+ } else {
+ return prefix;
+ }
+ }
+
+ public static boolean isPrefixedBy(final String columnName, final String prefix)
Review comment:
Works for me! I haven't gotten in the habit of looking out for those annotations yet, but I love the idea of NonnullBy default!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on a change in pull request #9235: Add
join-related DataSource types, and analysis functionality.
Posted by GitBox <gi...@apache.org>.
gianm commented on a change in pull request #9235: Add join-related DataSource types, and analysis functionality.
URL: https://github.com/apache/druid/pull/9235#discussion_r369345509
##########
File path: processing/src/test/java/org/apache/druid/query/planning/DataSourceAnalysisTest.java
##########
@@ -0,0 +1,476 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.planning;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.druid.java.util.common.Intervals;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.granularity.Granularities;
+import org.apache.druid.math.expr.ExprMacroTable;
+import org.apache.druid.query.DataSource;
+import org.apache.druid.query.InlineDataSource;
+import org.apache.druid.query.JoinDataSource;
+import org.apache.druid.query.LookupDataSource;
+import org.apache.druid.query.QueryDataSource;
+import org.apache.druid.query.TableDataSource;
+import org.apache.druid.query.UnionDataSource;
+import org.apache.druid.query.groupby.GroupByQuery;
+import org.apache.druid.query.spec.MultipleIntervalSegmentSpec;
+import org.apache.druid.segment.column.ValueType;
+import org.apache.druid.segment.join.JoinConditionAnalysis;
+import org.apache.druid.segment.join.JoinType;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Optional;
+
+public class DataSourceAnalysisTest
+{
+ private static final TableDataSource TABLE_FOO = new TableDataSource("foo");
+ private static final TableDataSource TABLE_BAR = new TableDataSource("bar");
+ private static final LookupDataSource LOOKUP_LOOKYLOO = new LookupDataSource("lookyloo");
+ private static final InlineDataSource INLINE = InlineDataSource.fromIterable(
+ ImmutableList.of("column"),
+ ImmutableList.of(ValueType.STRING),
+ ImmutableList.of(new Object[0])
+ );
+
+ @Test
+ public void testTable()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(TABLE_FOO);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(TABLE_FOO, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(unionDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(unionDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnTable()
+ {
+ final QueryDataSource queryDataSource = subquery(TABLE_FOO);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(TABLE_FOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.of(TABLE_FOO), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnUnion()
+ {
+ final UnionDataSource unionDataSource = new UnionDataSource(ImmutableList.of(TABLE_FOO, TABLE_BAR));
+ final QueryDataSource queryDataSource = subquery(unionDataSource);
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(queryDataSource);
+
+ Assert.assertTrue(analysis.isConcreteBased());
+ Assert.assertTrue(analysis.isConcreteTableBased());
+ Assert.assertFalse(analysis.isGlobal());
+ Assert.assertTrue(analysis.isQuery());
+ Assert.assertEquals(queryDataSource, analysis.getDataSource());
+ Assert.assertEquals(unionDataSource, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(
+ Optional.of(new MultipleIntervalSegmentSpec(ImmutableList.of(Intervals.of("2000/3000")))),
+ analysis.getBaseQuerySegmentSpec()
+ );
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testLookup()
+ {
+ final DataSourceAnalysis analysis = DataSourceAnalysis.forDataSource(LOOKUP_LOOKYLOO);
+
+ Assert.assertFalse(analysis.isConcreteBased());
+ Assert.assertFalse(analysis.isConcreteTableBased());
+ Assert.assertTrue(analysis.isGlobal());
+ Assert.assertFalse(analysis.isQuery());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getDataSource());
+ Assert.assertEquals(LOOKUP_LOOKYLOO, analysis.getBaseDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseTableDataSource());
+ Assert.assertEquals(Optional.empty(), analysis.getBaseQuerySegmentSpec());
+ Assert.assertEquals(Collections.emptyList(), analysis.getPreJoinableClauses());
+ }
+
+ @Test
+ public void testQueryOnLookup()
+ {
+ final QueryDataSource queryDataSource = new QueryDataSource(
+ GroupByQuery.builder()
+ .setDataSource(LOOKUP_LOOKYLOO)
+ .setInterval(new MultipleIntervalSegmentSpec(Collections.singletonList(Intervals.of("2000/3000"))))
+ .setGranularity(Granularities.ALL)
+ .build()
+ );
Review comment:
You're right, I'll change it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org