This is an automated email from the ASF dual-hosted git repository.
karan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new eb056e23b5e Fix dictionarySize overrides in tests (#15354)
eb056e23b5e is described below
commit eb056e23b5e7d9ff2b1ffc3d8731c818eae4fade
Author: Zoltan Haindrich <[email protected]>
AuthorDate: Tue Nov 28 14:19:09 2023 +0100
Fix dictionarySize overrides in tests (#15354)
I think this is a problem as it discards the false return value when the
putToKeyBuffer can't store the value because of the limit
Not forwarding the return value at that point may lead to the normal
continuation here regardless something was not added to the dictionary like here
---
docs/querying/groupbyquery.md | 2 ++
.../main/java/org/apache/druid/query/QueryContext.java | 5 +++++
.../apache/druid/query/groupby/GroupByQueryConfig.java | 15 +++++++++++----
.../groupby/epinephelinae/RowBasedGrouperHelper.java | 3 +--
.../groupby/epinephelinae/RowBasedKeySerdeHelper.java | 2 ++
.../column/GroupByColumnSelectorStrategy.java | 3 +++
.../druid/query/groupby/GroupByQueryConfigTest.java | 4 ++--
.../druid/sql/calcite/planner/DruidSqlValidator.java | 4 ++--
.../org/apache/druid/sql/calcite/CalciteQueryTest.java | 2 +-
9 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/docs/querying/groupbyquery.md b/docs/querying/groupbyquery.md
index d6980476c0b..935bd90a145 100644
--- a/docs/querying/groupbyquery.md
+++ b/docs/querying/groupbyquery.md
@@ -378,6 +378,8 @@ Supported query contexts:
|`forceHashAggregation`|Overrides the value of
`druid.query.groupBy.forceHashAggregation`|None|
|`intermediateCombineDegree`|Overrides the value of
`druid.query.groupBy.intermediateCombineDegree`|None|
|`numParallelCombineThreads`|Overrides the value of
`druid.query.groupBy.numParallelCombineThreads`|None|
+|`maxSelectorDictionarySize`|Overrides the value of
`druid.query.groupBy.maxMergingDictionarySize`|None|
+|`maxMergingDictionarySize`|Overrides the value of
`druid.query.groupBy.maxMergingDictionarySize`|None|
|`mergeThreadLocal`|Whether merge buffers should always be split into
thread-local buffers. Setting this to `true` reduces thread contention, but
uses memory less efficiently. This tradeoff is beneficial when memory is
plentiful. |false|
|`sortByDimsFirst`|Sort the results first by dimension values and then by
timestamp.|false|
|`forceLimitPushDown`|When all fields in the orderby are part of the grouping
key, the Broker will push limit application down to the Historical processes.
When the sorting order uses fields that are not in the grouping key, applying
this optimization can result in approximate results with unknown accuracy, so
this optimization is disabled by default in that case. Enabling this context
flag turns on limit push down for limit/orderbys that contain non-grouping key
columns.|false|
diff --git a/processing/src/main/java/org/apache/druid/query/QueryContext.java
b/processing/src/main/java/org/apache/druid/query/QueryContext.java
index c247ab0e639..6fa73f0b65c 100644
--- a/processing/src/main/java/org/apache/druid/query/QueryContext.java
+++ b/processing/src/main/java/org/apache/druid/query/QueryContext.java
@@ -217,6 +217,11 @@ public class QueryContext
return QueryContexts.getAsHumanReadableBytes(key, get(key), defaultValue);
}
+ public HumanReadableBytes getHumanReadableBytes(final String key, final long
defaultBytes)
+ {
+ return QueryContexts.getAsHumanReadableBytes(key, get(key),
HumanReadableBytes.valueOf(defaultBytes));
+ }
+
public <E extends Enum<E>> E getEnum(String key, Class<E> clazz, E
defaultValue)
{
return QueryContexts.getAsEnum(key, get(key), clazz, defaultValue);
diff --git
a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryConfig.java
b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryConfig.java
index 6b94ba3bc53..40dc9715885 100644
---
a/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryConfig.java
+++
b/processing/src/main/java/org/apache/druid/query/groupby/GroupByQueryConfig.java
@@ -48,6 +48,8 @@ public class GroupByQueryConfig
private static final String CTX_KEY_BUFFER_GROUPER_INITIAL_BUCKETS =
"bufferGrouperInitialBuckets";
private static final String CTX_KEY_BUFFER_GROUPER_MAX_LOAD_FACTOR =
"bufferGrouperMaxLoadFactor";
private static final String CTX_KEY_MAX_ON_DISK_STORAGE = "maxOnDiskStorage";
+ private static final String CTX_KEY_MAX_SELECTOR_DICTIONARY_SIZE =
"maxSelectorDictionarySize";
+ private static final String CTX_KEY_MAX_MERGING_DICTIONARY_SIZE =
"maxMergingDictionarySize";
private static final String CTX_KEY_FORCE_HASH_AGGREGATION =
"forceHashAggregation";
private static final String CTX_KEY_INTERMEDIATE_COMBINE_DEGREE =
"intermediateCombineDegree";
private static final String CTX_KEY_NUM_PARALLEL_COMBINE_THREADS =
"numParallelCombineThreads";
@@ -164,7 +166,7 @@ public class GroupByQueryConfig
*/
long getActualMaxSelectorDictionarySize(final long maxHeapSize, final int
numConcurrentQueries)
{
- if (maxSelectorDictionarySize.getBytes() == AUTOMATIC) {
+ if (getConfiguredMaxSelectorDictionarySize() == AUTOMATIC) {
final long heapForDictionaries = (long) (maxHeapSize *
SELECTOR_DICTIONARY_HEAP_FRACTION);
return Math.max(
@@ -175,7 +177,7 @@ public class GroupByQueryConfig
)
);
} else {
- return maxSelectorDictionarySize.getBytes();
+ return getConfiguredMaxSelectorDictionarySize();
}
}
@@ -321,8 +323,13 @@ public class GroupByQueryConfig
getMaxOnDiskStorage().getBytes()
)
);
- newConfig.maxSelectorDictionarySize = maxSelectorDictionarySize; // No
overrides
- newConfig.maxMergingDictionarySize = maxMergingDictionarySize; // No
overrides
+
+ newConfig.maxSelectorDictionarySize = queryContext
+ .getHumanReadableBytes(CTX_KEY_MAX_SELECTOR_DICTIONARY_SIZE,
getConfiguredMaxSelectorDictionarySize());
+
+ newConfig.maxMergingDictionarySize = queryContext
+ .getHumanReadableBytes(CTX_KEY_MAX_MERGING_DICTIONARY_SIZE,
getConfiguredMaxMergingDictionarySize());
+
newConfig.forcePushDownLimit =
queryContext.getBoolean(CTX_KEY_FORCE_LIMIT_PUSH_DOWN, isForcePushDownLimit());
newConfig.applyLimitPushDownToSegment = queryContext.getBoolean(
CTX_KEY_APPLY_LIMIT_PUSH_DOWN_TO_SEGMENT,
diff --git
a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java
b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java
index 8c3e485ef7c..1bdad6df088 100644
---
a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java
+++
b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedGrouperHelper.java
@@ -1921,8 +1921,7 @@ public class RowBasedGrouperHelper
} else {
keyBuffer.put(NullHandling.IS_NOT_NULL_BYTE);
}
- delegate.putToKeyBuffer(key, idx);
- return true;
+ return delegate.putToKeyBuffer(key, idx);
}
@Override
diff --git
a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedKeySerdeHelper.java
b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedKeySerdeHelper.java
index a65c2ea031c..1cb29d23bc0 100644
---
a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedKeySerdeHelper.java
+++
b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/RowBasedKeySerdeHelper.java
@@ -22,6 +22,7 @@ package org.apache.druid.query.groupby.epinephelinae;
import org.apache.druid.query.groupby.epinephelinae.Grouper.BufferComparator;
import
org.apache.druid.query.groupby.epinephelinae.RowBasedGrouperHelper.RowBasedKey;
+import javax.annotation.CheckReturnValue;
import java.nio.ByteBuffer;
interface RowBasedKeySerdeHelper
@@ -43,6 +44,7 @@ interface RowBasedKeySerdeHelper
*
* @return true if the value was added to the key, false otherwise
*/
+ @CheckReturnValue
boolean putToKeyBuffer(RowBasedKey key, int idx);
/**
diff --git
a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/column/GroupByColumnSelectorStrategy.java
b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/column/GroupByColumnSelectorStrategy.java
index 1a2bfc580a3..26095b5a2b2 100644
---
a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/column/GroupByColumnSelectorStrategy.java
+++
b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/column/GroupByColumnSelectorStrategy.java
@@ -25,6 +25,7 @@ import org.apache.druid.query.groupby.epinephelinae.Grouper;
import org.apache.druid.query.ordering.StringComparator;
import org.apache.druid.segment.ColumnValueSelector;
+import javax.annotation.CheckReturnValue;
import javax.annotation.Nullable;
import java.nio.ByteBuffer;
@@ -89,6 +90,7 @@ public interface GroupByColumnSelectorStrategy extends
ColumnSelectorStrategy
* @return estimated increase in internal state footprint, in bytes, as a
result of this operation. May be zero if
* memory did not increase as a result of this operation. Will not be
negative.
*/
+ @CheckReturnValue
int initColumnValues(ColumnValueSelector selector, int columnIndex, Object[]
valuess);
/**
@@ -144,6 +146,7 @@ public interface GroupByColumnSelectorStrategy extends
ColumnSelectorStrategy
* @return estimated increase in internal state footprint, in bytes, as a
result of this operation. May be zero if
* memory did not increase as a result of this operation. Will not be
negative.
*/
+ @CheckReturnValue
int writeToKeyBuffer(int keyBufferPosition, ColumnValueSelector selector,
ByteBuffer keyBuffer);
/**
diff --git
a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryConfigTest.java
b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryConfigTest.java
index b034210bcf7..34ed99cda21 100644
---
a/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryConfigTest.java
+++
b/processing/src/test/java/org/apache/druid/query/groupby/GroupByQueryConfigTest.java
@@ -102,8 +102,8 @@ public class GroupByQueryConfigTest
Assert.assertEquals(true, config2.isSingleThreaded());
Assert.assertEquals(1, config2.getBufferGrouperInitialBuckets());
Assert.assertEquals(3_000_000, config2.getMaxOnDiskStorage().getBytes());
- Assert.assertEquals(5 /* Can't override */,
config2.getConfiguredMaxSelectorDictionarySize());
- Assert.assertEquals(6_000_000 /* Can't override */,
config2.getConfiguredMaxMergingDictionarySize());
+ Assert.assertEquals(3, config2.getConfiguredMaxSelectorDictionarySize());
+ Assert.assertEquals(4, config2.getConfiguredMaxMergingDictionarySize());
Assert.assertEquals(7.0, config2.getBufferGrouperMaxLoadFactor(), 0.0);
Assert.assertTrue(config2.isApplyLimitPushDownToSegment());
}
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
index 2844350241b..34f3c7410f3 100644
---
a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
+++
b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidSqlValidator.java
@@ -60,8 +60,8 @@ class DruidSqlValidator extends BaseDruidSqlValidator
if (!plannerContext.featureAvailable(EngineFeature.WINDOW_FUNCTIONS)) {
throw buildCalciteContextException(
StringUtils.format(
- "The query contains window functions; To run these window
functions, enable [%s] in query context.",
- EngineFeature.WINDOW_FUNCTIONS),
+ "The query contains window functions; To run these window
functions, specify [%s] in query context.",
+ PlannerContext.CTX_ENABLE_WINDOW_FNS),
call);
}
}
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index eb232c6957a..e03130a04f1 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -14396,7 +14396,7 @@ public class CalciteQueryTest extends
BaseCalciteQueryTest
.sql("SELECT dim1,ROW_NUMBER() OVER () from druid.foo")
.run());
- assertThat(e, invalidSqlIs("The query contains window functions; To run
these window functions, enable [WINDOW_FUNCTIONS] in query context. (line [1],
column [13])"));
+ assertThat(e, invalidSqlIs("The query contains window functions; To run
these window functions, specify [enableWindowing] in query context. (line [1],
column [13])"));
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
