[PR] Moving bitwise_or to use native calcite operator (druid)
somu-imply opened a new pull request, #16237: URL: https://github.com/apache/druid/pull/16237 This PR has: - [ ] been self-reviewed. - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.) - [ ] added documentation for new or modified features or behaviors. - [ ] a release note entry in the PR description. - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links. - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md) - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met. - [ ] added integration tests. - [ ] been tested in a test Druid cluster. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[PR] Reduce upload buffer size in GoogleTaskLogs. (druid)
gianm opened a new pull request, #16236: URL: https://github.com/apache/druid/pull/16236 Use a 1MB upload buffer, rather than the default of 15 MB in the API client. This is mainly because MMs may upload logs in parallel, and typically have small heaps. The default-sized 15 MB buffers add up quickly and can cause a MM to run out of memory. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Allow typedIn to run in replace-with-default mode. (druid)
gianm commented on PR #16233: URL: https://github.com/apache/druid/pull/16233#issuecomment-2036099925 The legit failure is fixed; all that's left is the branch coverage failures, which I suggest we ignore. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[PR] Web console: fix query timer issues (druid)
vogievetsky opened a new pull request, #16235: URL: https://github.com/apache/druid/pull/16235 Fix several issues with the query timer in the console, specifically: - Timer is not shown if an error happens - Timer resets if changing tabs while query is running - Error state is lost if tab is switched twice  -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Allow typedIn to run in replace-with-default mode. (druid)
gianm commented on PR #16233: URL: https://github.com/apache/druid/pull/16233#issuecomment-2035655435 I think we should ignore the branch coverage check here, because the relevant branch is `if (NullHandling.sqlCompatible())`, which we actually _are_ testing both branches of— just not in the same run. There were some legit test failures in the non-SQL-compat `processing` run, which I've fixed in the most recent commit. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] DO NOT MERGE - [Docs] Fix API placeholder formatting (druid)
ektravel closed pull request #15872: DO NOT MERGE - [Docs] Fix API placeholder formatting URL: https://github.com/apache/druid/pull/15872 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Allow typedIn to run in replace-with-default mode. (druid)
pranavbhole commented on PR #16233: URL: https://github.com/apache/druid/pull/16233#issuecomment-2035323692 CI is failing due to low branch coverage. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Improve code flow in the First/Last vector aggregators and unify the numeric aggregators with the String implementations (druid)
github-advanced-security[bot] commented on code in PR #16230:
URL: https://github.com/apache/druid/pull/16230#discussion_r1550226607
##
processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstLastUtils.java:
##
@@ -38,25 +38,29 @@
* Return the object at a particular index from the vector selectors.
* index of bounds issues is the responsibility of the caller
*/
+ @Nullable
public static SerializablePairLongString readPairFromVectorSelectorsAtIndex(
- VectorValueSelector timeSelector,
- VectorObjectSelector valueSelector,
+ boolean[] timeNullityVector,
+ long[] timeVector,
+ Object[] maybeFoldedObjects,
int index
)
{
final long time;
final String string;
-final Object object = valueSelector.getObjectVector()[index];
+final Object object = maybeFoldedObjects[index];
Review Comment:
## Dereferenced variable may be null
Variable [maybeFoldedObjects](1) may be null at this access because of
[this](2) null argument.
Variable [maybeFoldedObjects](1) may be null at this access because of
[this](3) null argument.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7208)
##
processing/src/main/java/org/apache/druid/query/aggregation/first/StringFirstLastUtils.java:
##
@@ -38,25 +38,29 @@
* Return the object at a particular index from the vector selectors.
* index of bounds issues is the responsibility of the caller
*/
+ @Nullable
public static SerializablePairLongString readPairFromVectorSelectorsAtIndex(
- VectorValueSelector timeSelector,
- VectorObjectSelector valueSelector,
+ boolean[] timeNullityVector,
+ long[] timeVector,
+ Object[] maybeFoldedObjects,
int index
)
{
final long time;
final String string;
-final Object object = valueSelector.getObjectVector()[index];
+final Object object = maybeFoldedObjects[index];
+
if (object instanceof SerializablePairLongString) {
final SerializablePairLongString pair = (SerializablePairLongString)
object;
time = pair.lhs;
string = pair.rhs;
-} else if (object != null) {
- time = timeSelector.getLongVector()[index];
- string = DimensionHandlerUtils.convertObjectToString(object);
} else {
- // Don't aggregate nulls.
- return null;
+ if (timeNullityVector != null && timeNullityVector[index]) {
+// Donot aggregate pairs where time is unknown
+return null;
+ }
+ time = timeVector[index];
Review Comment:
## Dereferenced variable may be null
Variable [timeVector](1) may be null at this access because of [this](2)
null argument.
Variable [timeVector](1) may be null at this access because of [this](3)
null argument.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7209)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
[PR] Web console: don't send lookups to sampler (druid)
vogievetsky opened a new pull request, #16234: URL: https://github.com/apache/druid/pull/16234 Instead of sending transform expressions containing lookups to the sampler, which is guaranteed to give an error of lookup not found due to lookups not being loaded on the overlord, change the transform to a placeholder so it does not block the flow:   -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
(druid) 01/01: don't send lookups to sampler
This is an automated email from the ASF dual-hosted git repository.
vogievetsky pushed a commit to branch dont-send-lookups-to-sampler
in repository https://gitbox.apache.org/repos/asf/druid.git
commit d75ae964d6e429d134abcd4640d2c6ad60e19355
Author: Vadim Ogievetsky
AuthorDate: Wed Apr 3 10:38:51 2024 -0700
don't send lookups to sampler
---
web-console/src/utils/sampler.spec.ts | 48 -
web-console/src/utils/sampler.ts | 68 +--
2 files changed, 113 insertions(+), 3 deletions(-)
diff --git a/web-console/src/utils/sampler.spec.ts
b/web-console/src/utils/sampler.spec.ts
index 692a2d14143..b0aa6304f89 100644
--- a/web-console/src/utils/sampler.spec.ts
+++ b/web-console/src/utils/sampler.spec.ts
@@ -17,7 +17,7 @@
*/
import type { SampleResponse } from './sampler';
-import { guessDimensionsFromSampleResponse } from './sampler';
+import { changeLookupInExpressionsSampling, guessDimensionsFromSampleResponse
} from './sampler';
describe('sampler', () => {
describe('getInferredDimensionsFromSampleResponse', () => {
@@ -130,4 +130,50 @@ describe('sampler', () => {
`);
});
});
+
+ describe('changeLookupInExpressionsSampling', () => {
+it('does nothing when there is nothing to do', () => {
+ expect(changeLookupInExpressionsSampling(`concat("x",
'lol')`)).toEqual(`concat("x", 'lol')`);
+});
+
+it('works with a SQL parsable expression', () => {
+ expect(
+changeLookupInExpressionsSampling(`concat(lookup("x", 'lookup_name'),
'lol')`),
+ ).toEqual(
+`concat(concat('lookup_name', '[', "x", '] -- This is a placeholder,
lookups are not supported in sampling'), 'lol')`,
+ );
+
+ expect(
+changeLookupInExpressionsSampling(`concat(lookup("x", 'lookup_name',
'fallback'), 'lol')`),
+ ).toEqual(
+`concat(nvl(concat('lookup_name', '[', "x", '] -- This is a
placeholder, lookups are not supported in sampling'), 'fallback'), 'lol')`,
+ );
+
+ expect(
+changeLookupInExpressionsSampling(
+ `concat(lookup("x", 'lookup_name', 'fallback', '?'), 'lol')`,
+),
+ ).toEqual(`concat(null, 'lol')`);
+});
+
+it('works with a non-SQL parsable expression', () => {
+ expect(
+changeLookupInExpressionsSampling(`concat(lookup("x", 'lookup_name'),
'lol')^`),
+ ).toEqual(
+`concat(concat('lookup_name','[',"x",'] -- This is a placeholder,
lookups are not supported in sampling'), 'lol')^`,
+ );
+
+ expect(
+changeLookupInExpressionsSampling(`concat(lookup("x", 'lookup_name',
'fallback'), 'lol')^`),
+ ).toEqual(
+`concat(nvl(concat('lookup_name','[',"x",'] -- This is a placeholder,
lookups are not supported in sampling'),'fallback'), 'lol')^`,
+ );
+
+ expect(
+changeLookupInExpressionsSampling(
+ `concat(lookup("x", 'lookup_name', 'fallback', '?'), 'lol')^`,
+),
+ ).toEqual(`concat(null, 'lol')^`);
+});
+ });
});
diff --git a/web-console/src/utils/sampler.ts b/web-console/src/utils/sampler.ts
index 8b1d25320b7..28489c52400 100644
--- a/web-console/src/utils/sampler.ts
+++ b/web-console/src/utils/sampler.ts
@@ -16,7 +16,7 @@
* limitations under the License.
*/
-import { dedupe } from '@druid-toolkit/query';
+import { dedupe, F, SqlExpression, SqlFunction } from '@druid-toolkit/query';
import * as JSONBig from 'json-bigint-native';
import type {
@@ -186,7 +186,7 @@ export async function postToSampler(
sampleSpec: SampleSpec,
forStr: string,
): Promise {
- sampleSpec = fixSamplerTypes(sampleSpec);
+ sampleSpec = fixSamplerLookups(fixSamplerTypes(sampleSpec));
let sampleResp: any;
try {
@@ -622,3 +622,67 @@ export async function sampleForSchema(
return postToSampler(applyCache(sampleSpec, cacheRows), 'schema');
}
+
+function fixSamplerLookups(sampleSpec: SampleSpec): SampleSpec {
+ const transforms: Transform[] | undefined = deepGet(
+sampleSpec,
+'spec.dataSchema.transformSpec.transforms',
+ );
+ if (!Array.isArray(transforms)) return sampleSpec;
+
+ return deepSet(
+sampleSpec,
+'spec.dataSchema.transformSpec.transforms',
+transforms.map(transform => {
+ const { expression } = transform;
+ if (typeof expression !== 'string') return transform;
+ return { ...transform, expression:
changeLookupInExpressionsSampling(expression) };
+}),
+ );
+}
+
+/**
+ * Lookups do not work in the sampler because they are not loaded in the
Overlord
+ * to prevent the user from getting an error like "Unknown lookup [lookup
name]" we
+ * change the lookup expression to a placeholder
+ *
+ * lookup("x", 'lookup_name') => concat('lookup_name', '[', "x", '] -- This is
a placeholder, lookups are not supported in sampling')
+ * lookup("x", 'lookup_name', 'replaceValue') => nvl(concat('lookup_name',
'[', "x", '] -- This is a placeholder, lookups are not supported in sampling'),
(druid) branch dont-send-lookups-to-sampler created (now d75ae964d6e)
This is an automated email from the ASF dual-hosted git repository. vogievetsky pushed a change to branch dont-send-lookups-to-sampler in repository https://gitbox.apache.org/repos/asf/druid.git at d75ae964d6e don't send lookups to sampler This branch includes the following new commits: new d75ae964d6e don't send lookups to sampler The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
(druid) branch master updated: Update error messages when supervisor's checkpoint state is invalid (#16208)
This is an automated email from the ASF dual-hosted git repository.
abhishekrb 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 75fb57ed6e3 Update error messages when supervisor's checkpoint state
is invalid (#16208)
75fb57ed6e3 is described below
commit 75fb57ed6e36f2c30fc8aec070dca4762823a9c2
Author: Abhishek Radhakrishnan
AuthorDate: Wed Apr 3 10:34:17 2024 -0700
Update error messages when supervisor's checkpoint state is invalid (#16208)
* Update error message when topic messages.
Suggest resetting the supervisor when the topic changes instead of changing
the supervisor name which is actually making a new supervisor.
* Update
server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java
Co-authored-by: Kashif Faraz
* Cleanup
* Remove log and include oldCommitMetadataFromDb
* Fix test
-
Co-authored-by: Kashif Faraz
---
.../SegmentTransactionalInsertActionTest.java | 8 +-
.../IndexerSQLMetadataStorageCoordinator.java | 88 ++
.../IndexerSQLMetadataStorageCoordinatorTest.java | 35 ++---
3 files changed, 72 insertions(+), 59 deletions(-)
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentTransactionalInsertActionTest.java
b/indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentTransactionalInsertActionTest.java
index 6f8e827c705..847354706ba 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentTransactionalInsertActionTest.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentTransactionalInsertActionTest.java
@@ -22,6 +22,7 @@ package org.apache.druid.indexing.common.actions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import org.apache.druid.error.InvalidInput;
import org.apache.druid.indexing.common.TaskLockType;
import org.apache.druid.indexing.common.task.NoopTask;
import org.apache.druid.indexing.common.task.Task;
@@ -151,7 +152,12 @@ public class SegmentTransactionalInsertActionTest
);
Assert.assertEquals(
-SegmentPublishResult.fail("java.lang.RuntimeException: Failed to
update the metadata Store. The new start metadata is ahead of last commited end
state."),
+SegmentPublishResult.fail(
+InvalidInput.exception(
+"The new start metadata state[ObjectMetadata{theObject=[1]}]
is ahead of the last commited end"
++ " state[null]. Try resetting the supervisor."
+).toString()
+),
result
);
}
diff --git
a/server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java
b/server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java
index e2addccbcb9..d364299d21d 100644
---
a/server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java
+++
b/server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java
@@ -33,6 +33,7 @@ import com.google.common.hash.Hashing;
import com.google.common.io.BaseEncoding;
import com.google.inject.Inject;
import org.apache.commons.lang.StringEscapeUtils;
+import org.apache.druid.error.InvalidInput;
import org.apache.druid.indexing.overlord.DataSourceMetadata;
import org.apache.druid.indexing.overlord.IndexerMetadataStorageCoordinator;
import org.apache.druid.indexing.overlord.SegmentCreateRequest;
@@ -445,41 +446,33 @@ public class IndexerSQLMetadataStorageCoordinator
implements IndexerMetadataStor
try {
return connector.retryTransaction(
- new TransactionCallback()
- {
-@Override
-public SegmentPublishResult inTransaction(
-final Handle handle,
-final TransactionStatus transactionStatus
-) throws Exception
-{
- // Set definitelyNotUpdated back to false upon retrying.
- definitelyNotUpdated.set(false);
+ (handle, transactionStatus) -> {
+// Set definitelyNotUpdated back to false upon retrying.
+definitelyNotUpdated.set(false);
- if (startMetadata != null) {
-final DataStoreMetadataUpdateResult result =
updateDataSourceMetadataWithHandle(
-handle,
-dataSource,
-startMetadata,
-endMetadata
-);
-
-if (result.isFailed()) {
- // Metadata was definitely not updated.
- transactionStatus.setRollbackOnly();
- definitelyNotUpdated.set(true);
-
- if
Re: [PR] Update error messages when supervisor's checkpoint state is invalid (druid)
abhishekrb19 merged PR #16208: URL: https://github.com/apache/druid/pull/16208 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Allow typedIn to run in replace-with-default mode. (druid)
gianm commented on PR #16233: URL: https://github.com/apache/druid/pull/16233#issuecomment-2035052561 I tested this locally by running a Broker in SQL-compatible mode and a Historical in replace-with-default mode. In this case, the Broker generates `inType` filters. Without this patch, the Historical threw an error on the query; with this patch, the query ran successfully. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Fixing return type for IPV4 (druid)
pranavbhole commented on PR #15916: URL: https://github.com/apache/druid/pull/15916#issuecomment-2034914438 @somu-imply Can we rebase and merge 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. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)
LakshSingla commented on code in PR #16130: URL: https://github.com/apache/druid/pull/16130#discussion_r1549763794 ## extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/error/FrameFieldWriterFault.java: ## Review Comment: Let's add this class to `MSQFaultsSerdeTest` for completeness. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Record column name for exceptions while writing frames in RowBasedFrameWriter (druid)
LakshSingla commented on code in PR #16130:
URL: https://github.com/apache/druid/pull/16130#discussion_r1549761358
##
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerImpl.java:
##
@@ -2983,6 +2984,20 @@ private MSQErrorReport
mapQueryColumnNameToOutputColumnName(
.build(),
task.getQuerySpec().getColumnMappings()
);
+} else if (workerErrorReport.getFault() instanceof
FrameFieldWriterException) {
+ FrameFieldWriterException ffre = (FrameFieldWriterException)
workerErrorReport.getFault();
+ return MSQErrorReport.fromException(
+ workerErrorReport.getTaskId(),
+ workerErrorReport.getHost(),
+ workerErrorReport.getStageNumber(),
+ FrameFieldWriterException.builder()
+ .source(ffre.getSource())
+ .rowNumber(ffre.getRowNumber())
+ .column(ffre.getColumn())
+ .errorMsg(ffre.getErrorMsg())
+ .build(),
Review Comment:
Let's use the normal constructor since we are using all the parameters in a
single place. Invalid null byte exception has the builder because it populates
different information at different places up the call stack.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] handling window frame selection in case of same bound kind (druid)
kgyrtkirk commented on code in PR #16231: URL: https://github.com/apache/druid/pull/16231#discussion_r1549450440 ## processing/src/main/java/org/apache/druid/query/rowsandcols/semantic/DefaultFramedOnHeapAggregatable.java: ## @@ -434,7 +434,9 @@ private AppendableRowsAndColumns computeCumulativeAggregates(AggregatorFactory[] // a specialized implementation of this interface against, say, a Frame object that can deal with arrays instead // of trying to optimize this generic implementation. Object[][] results = new Object[aggFactories.length][numRows]; -int resultStorageIndex = 0; + +// if the upper offset is -ve, then we need to ignore those many rows prior to the current row +int resultStorageIndex = -1 * Math.min(upperOffset, 0); Review Comment: nit: why not use an unary `-` ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] handling window frame selection in case of same bound kind (druid)
kgyrtkirk commented on code in PR #16231: URL: https://github.com/apache/druid/pull/16231#discussion_r1549445767 ## processing/src/main/java/org/apache/druid/query/rowsandcols/semantic/DefaultFramedOnHeapAggregatable.java: ## @@ -187,7 +187,7 @@ public AggInterval next() groupToRowIndex(relativeGroupId(1)) ), Interval.of( - groupToRowIndex(relativeGroupId(-lowerOffset)), + groupToRowIndex(relativeGroupId(lowerOffset < 0 ? lowerOffset : -lowerOffset)), Review Comment: this doesn't look right - this is effectively: `-Math.abs(lowerOffset)` this part was already working with relative offsets ; so I think it should be simply `relativeGroupId(lowerOffset)` - didn't that worked? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
(druid) branch master updated (4bea865697e -> 1df41db46d2)
This is an automated email from the ASF dual-hosted git repository. kgyrtkirk pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/druid.git from 4bea865697e Restore context flag for window functions (#16229) add 1df41db46d2 Migrate to use docker compose v2 (#16232) No new revisions were added by this update. Summary of changes: distribution/docker/README.md | 2 +- docs/tutorials/docker.md | 4 ++-- extensions-contrib/opentelemetry-emitter/README.md | 2 +- integration-tests-ex/cases/cluster.sh | 6 +++--- integration-tests/README.md| 20 ++-- integration-tests/script/docker_run_cluster.sh | 6 +++--- integration-tests/stop_cluster.sh | 6 +++--- 7 files changed, 23 insertions(+), 23 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Migrate to use docker compose v2 (druid)
kgyrtkirk merged PR #16232: URL: https://github.com/apache/druid/pull/16232 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
(druid) branch master updated (218513ad559 -> 4bea865697e)
This is an automated email from the ASF dual-hosted git repository. karan pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/druid.git from 218513ad559 Use created time from metadata store in list tasks (#16228) add 4bea865697e Restore context flag for window functions (#16229) No new revisions were added by this update. Summary of changes: .../hll/sql/HllSketchSqlAggregatorTest.java | 3 +++ .../java/org/apache/druid/msq/test/MSQTestBase.java | 2 ++ .../sql/calcite/planner/DruidSqlValidator.java | 10 ++ .../druid/sql/calcite/planner/PlannerContext.java | 10 ++ .../apache/druid/sql/calcite/CalciteQueryTest.java | 21 + .../druid/sql/calcite/CalciteSysQueryTest.java | 3 +++ .../druid/sql/calcite/CalciteWindowQueryTest.java | 3 +++ .../druid/sql/calcite/DrillWindowQueryTest.java | 9 ++--- 8 files changed, 58 insertions(+), 3 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Restore context flag for window functions (druid)
cryptoe merged PR #16229: URL: https://github.com/apache/druid/pull/16229 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Restore context flag for window functions (druid)
cryptoe commented on PR #16229: URL: https://github.com/apache/druid/pull/16229#issuecomment-2033898071 Since this is STS blocker, going ahead with merge. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
Re: [PR] Restore context flag for window functions (druid)
cryptoe commented on PR #16229: URL: https://github.com/apache/druid/pull/16229#issuecomment-2033897524 Changes LGTM. Though I would love a UT on the MSQ side which fails in case the flag is not set. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[PR] Migrate to use docker compose v2 (druid)
kgyrtkirk opened a new pull request, #16232: URL: https://github.com/apache/druid/pull/16232 fix `docker-compose` command failure https://github.com/actions/runner-images/issues/9557 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
svn commit: r68287 - in /release/druid: 24.0.2/ 28.0.0/ 29.0.0/
Author: karan Date: Wed Apr 3 07:35:42 2024 New Revision: 68287 Log: Removing older releses Removed: release/druid/24.0.2/ release/druid/28.0.0/ release/druid/29.0.0/ - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
[PR] handling window frame selection in case of same bound kind (druid)
sreemanamala opened a new pull request, #16231: URL: https://github.com/apache/druid/pull/16231 ### Description This PR aims to handle the following cases in window frame selection `rows between unbounded preceding and x preceding rows between x following and unbounded following rows between x preceding and y preceding rows between x following and y following` # Key changed/added classes in this PR * `DefaultFramedOnHeapAggregatable.java` * `DruidSqlValidator.java` * `Windowing.java` This PR has: - [ ] been self-reviewed. - [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.) - [ ] added documentation for new or modified features or behaviors. - [ ] a release note entry in the PR description. - [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links. - [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md) - [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met. - [ ] added integration tests. - [ ] been tested in a test Druid cluster. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
