github-advanced-security[bot] commented on code in PR #16985:
URL: https://github.com/apache/druid/pull/16985#discussion_r1740468884
##########
processing/src/main/java/org/apache/druid/segment/Segment.java:
##########
@@ -44,18 +52,114 @@
@Nullable
QueryableIndex asQueryableIndex();
- StorageAdapter asStorageAdapter();
+ @Deprecated
+ default StorageAdapter asStorageAdapter()
+ {
+ throw DruidException.defensive(
+ "asStorageAdapter is no longer supported, use Segment.asCursorFactory
to build cursors, or Segment.as(..) to get various metadata information instead"
+ );
+ }
+
+ default CursorFactory asCursorFactory()
+ {
+ // todo (clint): should we just tear off the bandaid and make everyone
implement this?
+ final StorageAdapter storageAdapter = asStorageAdapter();
Review Comment:
## Deprecated method or constructor invocation
Invoking [Segment.asStorageAdapter](1) should be avoided because it has been
deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7768)
##########
processing/src/main/java/org/apache/druid/segment/Segment.java:
##########
@@ -44,18 +52,114 @@
@Nullable
QueryableIndex asQueryableIndex();
- StorageAdapter asStorageAdapter();
+ @Deprecated
+ default StorageAdapter asStorageAdapter()
+ {
+ throw DruidException.defensive(
+ "asStorageAdapter is no longer supported, use Segment.asCursorFactory
to build cursors, or Segment.as(..) to get various metadata information instead"
+ );
+ }
+
+ default CursorFactory asCursorFactory()
+ {
+ // todo (clint): should we just tear off the bandaid and make everyone
implement this?
+ final StorageAdapter storageAdapter = asStorageAdapter();
+ return new CursorFactory()
+ {
+ @Override
+ public CursorHolder makeCursorHolder(CursorBuildSpec spec)
+ {
+ // For backwards compatibility, the default implementation assumes the
underlying rows are sorted by __time.
+ // Built-in implementations of StorageAdapter must override this
method.
+ final List<OrderBy> ordering;
+ final boolean descending;
+ if (Cursors.preferDescendingTimeOrdering(spec)) {
+ ordering = Cursors.descendingTimeOrder();
+ descending = true;
+ } else {
+ ordering = Cursors.ascendingTimeOrder();
+ descending = false;
+ }
+ return new CursorHolder()
+ {
+ @Override
+ public boolean canVectorize()
+ {
+ return storageAdapter.canVectorize(
+ spec.getFilter(),
+ spec.getVirtualColumns(),
+ descending
+ );
Review Comment:
## Deprecated method or constructor invocation
Invoking [StorageAdapter.canVectorize](1) should be avoided because it has
been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7769)
##########
processing/src/main/java/org/apache/druid/segment/Segment.java:
##########
@@ -44,18 +52,114 @@
@Nullable
QueryableIndex asQueryableIndex();
- StorageAdapter asStorageAdapter();
+ @Deprecated
+ default StorageAdapter asStorageAdapter()
+ {
+ throw DruidException.defensive(
+ "asStorageAdapter is no longer supported, use Segment.asCursorFactory
to build cursors, or Segment.as(..) to get various metadata information instead"
+ );
+ }
+
+ default CursorFactory asCursorFactory()
+ {
+ // todo (clint): should we just tear off the bandaid and make everyone
implement this?
+ final StorageAdapter storageAdapter = asStorageAdapter();
+ return new CursorFactory()
+ {
+ @Override
+ public CursorHolder makeCursorHolder(CursorBuildSpec spec)
+ {
+ // For backwards compatibility, the default implementation assumes the
underlying rows are sorted by __time.
+ // Built-in implementations of StorageAdapter must override this
method.
+ final List<OrderBy> ordering;
+ final boolean descending;
+ if (Cursors.preferDescendingTimeOrdering(spec)) {
+ ordering = Cursors.descendingTimeOrder();
+ descending = true;
+ } else {
+ ordering = Cursors.ascendingTimeOrder();
+ descending = false;
+ }
+ return new CursorHolder()
+ {
+ @Override
+ public boolean canVectorize()
+ {
+ return storageAdapter.canVectorize(
+ spec.getFilter(),
+ spec.getVirtualColumns(),
+ descending
+ );
+ }
+
+ @Override
+ public Cursor asCursor()
+ {
+ return Iterables.getOnlyElement(
+ storageAdapter.makeCursors(
+ spec.getFilter(),
+ spec.getInterval(),
+ spec.getVirtualColumns(),
+ Granularities.ALL,
+ descending,
+ spec.getQueryMetrics()
+ ).toList()
+ );
+ }
+
+ @Override
+ public VectorCursor asVectorCursor()
+ {
+ return storageAdapter.makeVectorCursor(
+ spec.getFilter(),
+ spec.getInterval(),
+ spec.getVirtualColumns(),
+ descending,
+ spec.getQueryContext().getVectorSize(),
+ spec.getQueryMetrics()
+ );
Review Comment:
## Deprecated method or constructor invocation
Invoking [StorageAdapter.makeVectorCursor](1) should be avoided because it
has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7771)
##########
processing/src/main/java/org/apache/druid/segment/Segment.java:
##########
@@ -44,18 +52,114 @@
@Nullable
QueryableIndex asQueryableIndex();
- StorageAdapter asStorageAdapter();
+ @Deprecated
+ default StorageAdapter asStorageAdapter()
+ {
+ throw DruidException.defensive(
+ "asStorageAdapter is no longer supported, use Segment.asCursorFactory
to build cursors, or Segment.as(..) to get various metadata information instead"
+ );
+ }
+
+ default CursorFactory asCursorFactory()
+ {
+ // todo (clint): should we just tear off the bandaid and make everyone
implement this?
+ final StorageAdapter storageAdapter = asStorageAdapter();
+ return new CursorFactory()
+ {
+ @Override
+ public CursorHolder makeCursorHolder(CursorBuildSpec spec)
+ {
+ // For backwards compatibility, the default implementation assumes the
underlying rows are sorted by __time.
+ // Built-in implementations of StorageAdapter must override this
method.
+ final List<OrderBy> ordering;
+ final boolean descending;
+ if (Cursors.preferDescendingTimeOrdering(spec)) {
+ ordering = Cursors.descendingTimeOrder();
+ descending = true;
+ } else {
+ ordering = Cursors.ascendingTimeOrder();
+ descending = false;
+ }
+ return new CursorHolder()
+ {
+ @Override
+ public boolean canVectorize()
+ {
+ return storageAdapter.canVectorize(
+ spec.getFilter(),
+ spec.getVirtualColumns(),
+ descending
+ );
+ }
+
+ @Override
+ public Cursor asCursor()
+ {
+ return Iterables.getOnlyElement(
+ storageAdapter.makeCursors(
+ spec.getFilter(),
+ spec.getInterval(),
+ spec.getVirtualColumns(),
+ Granularities.ALL,
+ descending,
+ spec.getQueryMetrics()
+ ).toList()
+ );
+ }
+
+ @Override
+ public VectorCursor asVectorCursor()
+ {
+ return storageAdapter.makeVectorCursor(
+ spec.getFilter(),
+ spec.getInterval(),
+ spec.getVirtualColumns(),
+ descending,
+ spec.getQueryContext().getVectorSize(),
+ spec.getQueryMetrics()
+ );
+ }
+
+ @Nullable
+ @Override
+ public List<OrderBy> getOrdering()
+ {
+ return ordering;
+ }
+
+ @Override
+ public void close()
+ {
+ // consuming sequences of CursorFactory are expected to close
themselves.
+ }
+ };
+ }
+
+ @Override
+ public RowSignature getRowSignature()
+ {
+ return storageAdapter.getRowSignature();
Review Comment:
## Deprecated method or constructor invocation
Invoking [StorageAdapter.getRowSignature](1) should be avoided because it
has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7772)
##########
processing/src/main/java/org/apache/druid/segment/Segment.java:
##########
@@ -44,18 +52,114 @@
@Nullable
QueryableIndex asQueryableIndex();
- StorageAdapter asStorageAdapter();
+ @Deprecated
+ default StorageAdapter asStorageAdapter()
+ {
+ throw DruidException.defensive(
+ "asStorageAdapter is no longer supported, use Segment.asCursorFactory
to build cursors, or Segment.as(..) to get various metadata information instead"
+ );
+ }
+
+ default CursorFactory asCursorFactory()
+ {
+ // todo (clint): should we just tear off the bandaid and make everyone
implement this?
+ final StorageAdapter storageAdapter = asStorageAdapter();
+ return new CursorFactory()
+ {
+ @Override
+ public CursorHolder makeCursorHolder(CursorBuildSpec spec)
+ {
+ // For backwards compatibility, the default implementation assumes the
underlying rows are sorted by __time.
+ // Built-in implementations of StorageAdapter must override this
method.
+ final List<OrderBy> ordering;
+ final boolean descending;
+ if (Cursors.preferDescendingTimeOrdering(spec)) {
+ ordering = Cursors.descendingTimeOrder();
+ descending = true;
+ } else {
+ ordering = Cursors.ascendingTimeOrder();
+ descending = false;
+ }
+ return new CursorHolder()
+ {
+ @Override
+ public boolean canVectorize()
+ {
+ return storageAdapter.canVectorize(
+ spec.getFilter(),
+ spec.getVirtualColumns(),
+ descending
+ );
+ }
+
+ @Override
+ public Cursor asCursor()
+ {
+ return Iterables.getOnlyElement(
+ storageAdapter.makeCursors(
+ spec.getFilter(),
+ spec.getInterval(),
+ spec.getVirtualColumns(),
+ Granularities.ALL,
+ descending,
+ spec.getQueryMetrics()
+ ).toList()
Review Comment:
## Deprecated method or constructor invocation
Invoking [StorageAdapter.makeCursors](1) should be avoided because it has
been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7770)
##########
processing/src/main/java/org/apache/druid/segment/Segment.java:
##########
@@ -44,18 +52,114 @@
@Nullable
QueryableIndex asQueryableIndex();
- StorageAdapter asStorageAdapter();
+ @Deprecated
+ default StorageAdapter asStorageAdapter()
+ {
+ throw DruidException.defensive(
+ "asStorageAdapter is no longer supported, use Segment.asCursorFactory
to build cursors, or Segment.as(..) to get various metadata information instead"
+ );
+ }
+
+ default CursorFactory asCursorFactory()
+ {
+ // todo (clint): should we just tear off the bandaid and make everyone
implement this?
+ final StorageAdapter storageAdapter = asStorageAdapter();
+ return new CursorFactory()
+ {
+ @Override
+ public CursorHolder makeCursorHolder(CursorBuildSpec spec)
+ {
+ // For backwards compatibility, the default implementation assumes the
underlying rows are sorted by __time.
+ // Built-in implementations of StorageAdapter must override this
method.
+ final List<OrderBy> ordering;
+ final boolean descending;
+ if (Cursors.preferDescendingTimeOrdering(spec)) {
+ ordering = Cursors.descendingTimeOrder();
+ descending = true;
+ } else {
+ ordering = Cursors.ascendingTimeOrder();
+ descending = false;
+ }
+ return new CursorHolder()
+ {
+ @Override
+ public boolean canVectorize()
+ {
+ return storageAdapter.canVectorize(
+ spec.getFilter(),
+ spec.getVirtualColumns(),
+ descending
+ );
+ }
+
+ @Override
+ public Cursor asCursor()
+ {
+ return Iterables.getOnlyElement(
+ storageAdapter.makeCursors(
+ spec.getFilter(),
+ spec.getInterval(),
+ spec.getVirtualColumns(),
+ Granularities.ALL,
+ descending,
+ spec.getQueryMetrics()
+ ).toList()
+ );
+ }
+
+ @Override
+ public VectorCursor asVectorCursor()
+ {
+ return storageAdapter.makeVectorCursor(
+ spec.getFilter(),
+ spec.getInterval(),
+ spec.getVirtualColumns(),
+ descending,
+ spec.getQueryContext().getVectorSize(),
+ spec.getQueryMetrics()
+ );
+ }
+
+ @Nullable
+ @Override
+ public List<OrderBy> getOrdering()
+ {
+ return ordering;
+ }
+
+ @Override
+ public void close()
+ {
+ // consuming sequences of CursorFactory are expected to close
themselves.
+ }
+ };
+ }
+
+ @Override
+ public RowSignature getRowSignature()
+ {
+ return storageAdapter.getRowSignature();
+ }
+
+ @Override
+ @Nullable
+ public ColumnCapabilities getColumnCapabilities(String column)
+ {
+ return storageAdapter.getColumnCapabilities(column);
Review Comment:
## Deprecated method or constructor invocation
Invoking [StorageAdapter.getColumnCapabilities](1) should be avoided because
it has been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7773)
##########
processing/src/test/java/org/apache/druid/frame/processor/FrameProcessorExecutorTest.java:
##########
@@ -144,14 +145,14 @@
final ListenableFuture<Long> blasterFuture = exec.runFully(blaster,
null);
final ListenableFuture<Long> muxerFuture = exec.runFully(muxer, null);
- Assert.assertEquals(adapter.getNumRows(), (long) blasterFuture.get());
- Assert.assertEquals(adapter.getNumRows() * 2, (long) muxerFuture.get());
+ Assert.assertEquals(index.size(), (long) blasterFuture.get());
+ Assert.assertEquals(index.size() * 2, (long) muxerFuture.get());
Assert.assertEquals(
- adapter.getNumRows() * 2,
+ index.size() * 2,
Review Comment:
## Result of multiplication cast to wider type
Potential overflow in [int multiplication](1) before it is converted to long
by use in an invocation context.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7766)
##########
processing/src/test/java/org/apache/druid/frame/processor/FrameProcessorExecutorTest.java:
##########
@@ -144,14 +145,14 @@
final ListenableFuture<Long> blasterFuture = exec.runFully(blaster,
null);
final ListenableFuture<Long> muxerFuture = exec.runFully(muxer, null);
- Assert.assertEquals(adapter.getNumRows(), (long) blasterFuture.get());
- Assert.assertEquals(adapter.getNumRows() * 2, (long) muxerFuture.get());
+ Assert.assertEquals(index.size(), (long) blasterFuture.get());
+ Assert.assertEquals(index.size() * 2, (long) muxerFuture.get());
Review Comment:
## Result of multiplication cast to wider type
Potential overflow in [int multiplication](1) before it is converted to long
by use in an invocation context.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7765)
##########
processing/src/test/java/org/apache/druid/query/lookup/LookupSegmentTest.java:
##########
@@ -234,8 +190,8 @@
@Test
public void test_asStorageAdapter_isRowBasedAdapter()
{
- // This allows us to assume that RowBasedStorageAdapterTest is further
exercising makeCursor and verifying misc.
+ // This allows us to assume that LookupSegmentTest is further exercising
makeCursor and verifying misc.
// methods like getMinTime, getMaxTime, getMetadata, etc, without checking
them explicitly in _this_ test class.
- Assert.assertThat(LOOKUP_SEGMENT.asStorageAdapter(),
CoreMatchers.instanceOf(RowBasedStorageAdapter.class));
+ Assert.assertThat(LOOKUP_SEGMENT.asCursorFactory(),
CoreMatchers.instanceOf(RowBasedCursorFactory.class));
Review Comment:
## Deprecated method or constructor invocation
Invoking [Assert.assertThat](1) should be avoided because it has been
deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7776)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
