github-advanced-security[bot] commented on code in PR #16533:
URL: https://github.com/apache/druid/pull/16533#discussion_r1693541054
##########
processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java:
##########
@@ -280,26 +279,24 @@
sortColumns
);
- final FrameWriter frameWriter =
frameWriterFactory.newFrameWriter(columnSelectorFactory);
- for (; !in.isDoneOrInterrupted() && remainingRowsToSkip > 0;
remainingRowsToSkip--) {
- in.advance();
+ final FrameWriter writer =
frameWriterFactory.newFrameWriter(columnSelectorFactory);
+ for (; !cursor.isDoneOrInterrupted() && remainingRowsToSkip > 0;
remainingRowsToSkip--) {
+ cursor.advance();
}
- for (; !in.isDoneOrInterrupted() && remainingRowsToFetch > 0;
remainingRowsToFetch--) {
- frameWriter.addSelection();
- in.advance();
+ for (; !cursor.isDoneOrInterrupted() && remainingRowsToFetch > 0;
remainingRowsToFetch--) {
+ writer.addSelection();
Review Comment:
## Dereferenced variable may be null
Variable [writer](1) may be null at this access as suggested by [this](2)
null guard.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7605)
##########
processing/src/main/java/org/apache/druid/query/rowsandcols/StorageAdapterRowsAndColumns.java:
##########
@@ -98,45 +95,35 @@
@Nonnull
private static RowsAndColumns materialize(StorageAdapter as)
{
- final Sequence<Cursor> cursors = as.makeCursors(
- null,
- Intervals.ETERNITY,
- VirtualColumns.EMPTY,
- Granularities.ALL,
- false,
- null
- );
-
- RowSignature rowSignature = as.getRowSignature();
-
- FrameWriter writer = cursors.accumulate(null, (accumulated, in) -> {
- if (accumulated != null) {
- // We should not get multiple cursors because we set the granularity
to ALL. So, this should never
- // actually happen, but it doesn't hurt us to defensive here, so we
test against it.
- throw new ISE("accumulated[%s] non-null, why did we get multiple
cursors?", accumulated);
+ try (final CursorMaker maker =
as.asCursorMaker(CursorBuildSpec.FULL_SCAN)) {
+ final Cursor cursor = maker.makeCursor();
+
+ if (cursor == null) {
+ return new EmptyRowsAndColumns();
}
+
+ final RowSignature rowSignature = as.getRowSignature();
- final ColumnSelectorFactory columnSelectorFactory =
in.getColumnSelectorFactory();
+ final ColumnSelectorFactory columnSelectorFactory =
cursor.getColumnSelectorFactory();
final FrameWriterFactory frameWriterFactory =
FrameWriters.makeColumnBasedFrameWriterFactory(
new ArenaMemoryAllocatorFactory(200 << 20), // 200 MB, because, why
not?
rowSignature,
Collections.emptyList()
);
- final FrameWriter frameWriter =
frameWriterFactory.newFrameWriter(columnSelectorFactory);
- while (!in.isDoneOrInterrupted()) {
- frameWriter.addSelection();
- in.advance();
+ final FrameWriter writer =
frameWriterFactory.newFrameWriter(columnSelectorFactory);
+ while (!cursor.isDoneOrInterrupted()) {
+ writer.addSelection();
Review Comment:
## Dereferenced variable may be null
Variable [writer](1) may be null at this access as suggested by [this](2)
null guard.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7606)
##########
processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVectorSelectorsTest.java:
##########
@@ -234,109 +235,100 @@
)
);
final QueryableIndexStorageAdapter storageAdapter = new
QueryableIndexStorageAdapter(index);
- VectorCursor cursor = storageAdapter.makeVectorCursor(
- null,
- index.getDataInterval(),
- virtualColumns,
- false,
- 512,
- null
- );
-
- ColumnCapabilities capabilities =
virtualColumns.getColumnCapabilitiesWithFallback(storageAdapter, "v");
-
- int rowCount = 0;
- if (capabilities.isDictionaryEncoded().isTrue()) {
- SingleValueDimensionVectorSelector selector =
cursor.getColumnSelectorFactory().makeSingleValueDimensionSelector(
- DefaultDimensionSpec.of("v")
- );
- while (!cursor.isDone()) {
- int[] row = selector.getRowVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++, rowCount++) {
- results.add(selector.lookupName(row[i]));
+ final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
+
.setInterval(index.getDataInterval())
+
.setGranularity(Granularities.ALL)
+
.setVirtualColumns(virtualColumns)
+ .build();
+ try (final CursorMaker maker = storageAdapter.asCursorMaker(buildSpec)) {
+ final VectorCursor cursor = maker.makeVectorCursor();
+
+ ColumnCapabilities capabilities =
virtualColumns.getColumnCapabilitiesWithFallback(storageAdapter, "v");
+
+ int rowCount = 0;
+ if (capabilities.isDictionaryEncoded().isTrue()) {
+ SingleValueDimensionVectorSelector selector =
cursor.getColumnSelectorFactory()
+
.makeSingleValueDimensionSelector(
+
DefaultDimensionSpec.of("v")
+ );
+ while (!cursor.isDone()) {
+ int[] row = selector.getRowVector();
+ for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
+ results.add(selector.lookupName(row[i]));
+ }
+ cursor.advance();
}
- cursor.advance();
- }
- } else {
- VectorValueSelector selector = null;
- VectorObjectSelector objectSelector = null;
- if (Types.isNumeric(outputType)) {
- selector = cursor.getColumnSelectorFactory().makeValueSelector("v");
} else {
- objectSelector =
cursor.getColumnSelectorFactory().makeObjectSelector("v");
- }
- GroupByVectorColumnSelector groupBySelector =
-
cursor.getColumnSelectorFactory().makeGroupByVectorColumnSelector("v",
DeferExpressionDimensions.ALWAYS);
- while (!cursor.isDone()) {
- final List<Object> resultsVector = new ArrayList<>();
- boolean[] nulls;
- switch (outputType.getType()) {
- case LONG:
- nulls = selector.getNullVector();
- long[] longs = selector.getLongVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null : longs[i]);
- }
- break;
- case DOUBLE:
- // special case to test floats just to get coverage on
getFloatVector
- if ("float2".equals(expression)) {
+ VectorValueSelector selector = null;
+ VectorObjectSelector objectSelector = null;
+ if (Types.isNumeric(outputType)) {
+ selector = cursor.getColumnSelectorFactory().makeValueSelector("v");
+ } else {
+ objectSelector =
cursor.getColumnSelectorFactory().makeObjectSelector("v");
+ }
+ GroupByVectorColumnSelector groupBySelector =
+
cursor.getColumnSelectorFactory().makeGroupByVectorColumnSelector("v",
DeferExpressionDimensions.ALWAYS);
+ while (!cursor.isDone()) {
+ final List<Object> resultsVector = new ArrayList<>();
+ boolean[] nulls;
+ switch (outputType.getType()) {
+ case LONG:
nulls = selector.getNullVector();
- float[] floats = selector.getFloatVector();
+ long[] longs = selector.getLongVector();
for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null : (double)
floats[i]);
+ resultsVector.add(nulls != null && nulls[i] ? null : longs[i]);
}
- } else {
- nulls = selector.getNullVector();
- double[] doubles = selector.getDoubleVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null :
doubles[i]);
+ break;
+ case DOUBLE:
+ // special case to test floats just to get coverage on
getFloatVector
+ if ("float2".equals(expression)) {
+ nulls = selector.getNullVector();
Review Comment:
## Dereferenced variable may be null
Variable [selector](1) may be null at this access because of [this](2)
assignment.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7609)
##########
processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVectorSelectorsTest.java:
##########
@@ -234,109 +235,100 @@
)
);
final QueryableIndexStorageAdapter storageAdapter = new
QueryableIndexStorageAdapter(index);
- VectorCursor cursor = storageAdapter.makeVectorCursor(
- null,
- index.getDataInterval(),
- virtualColumns,
- false,
- 512,
- null
- );
-
- ColumnCapabilities capabilities =
virtualColumns.getColumnCapabilitiesWithFallback(storageAdapter, "v");
-
- int rowCount = 0;
- if (capabilities.isDictionaryEncoded().isTrue()) {
- SingleValueDimensionVectorSelector selector =
cursor.getColumnSelectorFactory().makeSingleValueDimensionSelector(
- DefaultDimensionSpec.of("v")
- );
- while (!cursor.isDone()) {
- int[] row = selector.getRowVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++, rowCount++) {
- results.add(selector.lookupName(row[i]));
+ final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
+
.setInterval(index.getDataInterval())
+
.setGranularity(Granularities.ALL)
+
.setVirtualColumns(virtualColumns)
+ .build();
+ try (final CursorMaker maker = storageAdapter.asCursorMaker(buildSpec)) {
+ final VectorCursor cursor = maker.makeVectorCursor();
+
+ ColumnCapabilities capabilities =
virtualColumns.getColumnCapabilitiesWithFallback(storageAdapter, "v");
+
+ int rowCount = 0;
+ if (capabilities.isDictionaryEncoded().isTrue()) {
+ SingleValueDimensionVectorSelector selector =
cursor.getColumnSelectorFactory()
+
.makeSingleValueDimensionSelector(
+
DefaultDimensionSpec.of("v")
+ );
+ while (!cursor.isDone()) {
+ int[] row = selector.getRowVector();
+ for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
+ results.add(selector.lookupName(row[i]));
+ }
+ cursor.advance();
}
- cursor.advance();
- }
- } else {
- VectorValueSelector selector = null;
- VectorObjectSelector objectSelector = null;
- if (Types.isNumeric(outputType)) {
- selector = cursor.getColumnSelectorFactory().makeValueSelector("v");
} else {
- objectSelector =
cursor.getColumnSelectorFactory().makeObjectSelector("v");
- }
- GroupByVectorColumnSelector groupBySelector =
-
cursor.getColumnSelectorFactory().makeGroupByVectorColumnSelector("v",
DeferExpressionDimensions.ALWAYS);
- while (!cursor.isDone()) {
- final List<Object> resultsVector = new ArrayList<>();
- boolean[] nulls;
- switch (outputType.getType()) {
- case LONG:
- nulls = selector.getNullVector();
- long[] longs = selector.getLongVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null : longs[i]);
- }
- break;
- case DOUBLE:
- // special case to test floats just to get coverage on
getFloatVector
- if ("float2".equals(expression)) {
+ VectorValueSelector selector = null;
+ VectorObjectSelector objectSelector = null;
+ if (Types.isNumeric(outputType)) {
+ selector = cursor.getColumnSelectorFactory().makeValueSelector("v");
+ } else {
+ objectSelector =
cursor.getColumnSelectorFactory().makeObjectSelector("v");
+ }
+ GroupByVectorColumnSelector groupBySelector =
+
cursor.getColumnSelectorFactory().makeGroupByVectorColumnSelector("v",
DeferExpressionDimensions.ALWAYS);
+ while (!cursor.isDone()) {
+ final List<Object> resultsVector = new ArrayList<>();
+ boolean[] nulls;
+ switch (outputType.getType()) {
+ case LONG:
nulls = selector.getNullVector();
- float[] floats = selector.getFloatVector();
+ long[] longs = selector.getLongVector();
for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null : (double)
floats[i]);
+ resultsVector.add(nulls != null && nulls[i] ? null : longs[i]);
}
- } else {
- nulls = selector.getNullVector();
- double[] doubles = selector.getDoubleVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null :
doubles[i]);
+ break;
+ case DOUBLE:
+ // special case to test floats just to get coverage on
getFloatVector
+ if ("float2".equals(expression)) {
+ nulls = selector.getNullVector();
+ float[] floats = selector.getFloatVector();
+ for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
+ resultsVector.add(nulls != null && nulls[i] ? null :
(double) floats[i]);
+ }
+ } else {
+ nulls = selector.getNullVector();
Review Comment:
## Dereferenced variable may be null
Variable [selector](1) may be null at this access because of [this](2)
assignment.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7610)
##########
processing/src/main/java/org/apache/druid/segment/CursorFactory.java:
##########
@@ -33,34 +36,106 @@
*
* @see StorageAdapter
*/
-public interface CursorFactory
+public interface CursorFactory extends CursorMakerFactory
{
+ @Override
+ default CursorMaker asCursorMaker(CursorBuildSpec spec)
+ {
+ return new CursorMaker()
+ {
+ @Override
+ public boolean canVectorize()
+ {
+ return CursorFactory.this.canVectorize(spec.getFilter(),
spec.getVirtualColumns(), spec.isDescending());
+ }
+
+ @Override
+ public Cursor makeCursor()
+ {
+ return Iterables.getOnlyElement(
+ CursorFactory.this.makeCursors(
+ spec.getFilter(),
+ spec.getInterval(),
+ spec.getVirtualColumns(),
+ Granularities.ALL,
+ spec.isDescending(),
+ spec.getQueryMetrics()
+ ).toList()
Review Comment:
## Deprecated method or constructor invocation
Invoking [CursorFactory.makeCursors](1) should be avoided because it has
been deprecated.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7613)
##########
processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVectorSelectorsTest.java:
##########
@@ -234,109 +235,100 @@
)
);
final QueryableIndexStorageAdapter storageAdapter = new
QueryableIndexStorageAdapter(index);
- VectorCursor cursor = storageAdapter.makeVectorCursor(
- null,
- index.getDataInterval(),
- virtualColumns,
- false,
- 512,
- null
- );
-
- ColumnCapabilities capabilities =
virtualColumns.getColumnCapabilitiesWithFallback(storageAdapter, "v");
-
- int rowCount = 0;
- if (capabilities.isDictionaryEncoded().isTrue()) {
- SingleValueDimensionVectorSelector selector =
cursor.getColumnSelectorFactory().makeSingleValueDimensionSelector(
- DefaultDimensionSpec.of("v")
- );
- while (!cursor.isDone()) {
- int[] row = selector.getRowVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++, rowCount++) {
- results.add(selector.lookupName(row[i]));
+ final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
+
.setInterval(index.getDataInterval())
+
.setGranularity(Granularities.ALL)
+
.setVirtualColumns(virtualColumns)
+ .build();
+ try (final CursorMaker maker = storageAdapter.asCursorMaker(buildSpec)) {
+ final VectorCursor cursor = maker.makeVectorCursor();
+
+ ColumnCapabilities capabilities =
virtualColumns.getColumnCapabilitiesWithFallback(storageAdapter, "v");
+
+ int rowCount = 0;
+ if (capabilities.isDictionaryEncoded().isTrue()) {
+ SingleValueDimensionVectorSelector selector =
cursor.getColumnSelectorFactory()
+
.makeSingleValueDimensionSelector(
+
DefaultDimensionSpec.of("v")
+ );
+ while (!cursor.isDone()) {
+ int[] row = selector.getRowVector();
+ for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
+ results.add(selector.lookupName(row[i]));
+ }
+ cursor.advance();
}
- cursor.advance();
- }
- } else {
- VectorValueSelector selector = null;
- VectorObjectSelector objectSelector = null;
- if (Types.isNumeric(outputType)) {
- selector = cursor.getColumnSelectorFactory().makeValueSelector("v");
} else {
- objectSelector =
cursor.getColumnSelectorFactory().makeObjectSelector("v");
- }
- GroupByVectorColumnSelector groupBySelector =
-
cursor.getColumnSelectorFactory().makeGroupByVectorColumnSelector("v",
DeferExpressionDimensions.ALWAYS);
- while (!cursor.isDone()) {
- final List<Object> resultsVector = new ArrayList<>();
- boolean[] nulls;
- switch (outputType.getType()) {
- case LONG:
- nulls = selector.getNullVector();
- long[] longs = selector.getLongVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null : longs[i]);
- }
- break;
- case DOUBLE:
- // special case to test floats just to get coverage on
getFloatVector
- if ("float2".equals(expression)) {
+ VectorValueSelector selector = null;
+ VectorObjectSelector objectSelector = null;
+ if (Types.isNumeric(outputType)) {
+ selector = cursor.getColumnSelectorFactory().makeValueSelector("v");
+ } else {
+ objectSelector =
cursor.getColumnSelectorFactory().makeObjectSelector("v");
+ }
+ GroupByVectorColumnSelector groupBySelector =
+
cursor.getColumnSelectorFactory().makeGroupByVectorColumnSelector("v",
DeferExpressionDimensions.ALWAYS);
+ while (!cursor.isDone()) {
+ final List<Object> resultsVector = new ArrayList<>();
+ boolean[] nulls;
+ switch (outputType.getType()) {
+ case LONG:
nulls = selector.getNullVector();
- float[] floats = selector.getFloatVector();
+ long[] longs = selector.getLongVector();
for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null : (double)
floats[i]);
+ resultsVector.add(nulls != null && nulls[i] ? null : longs[i]);
}
- } else {
- nulls = selector.getNullVector();
- double[] doubles = selector.getDoubleVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null :
doubles[i]);
+ break;
+ case DOUBLE:
+ // special case to test floats just to get coverage on
getFloatVector
+ if ("float2".equals(expression)) {
+ nulls = selector.getNullVector();
+ float[] floats = selector.getFloatVector();
+ for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
+ resultsVector.add(nulls != null && nulls[i] ? null :
(double) floats[i]);
+ }
+ } else {
+ nulls = selector.getNullVector();
+ double[] doubles = selector.getDoubleVector();
+ for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
+ resultsVector.add(nulls != null && nulls[i] ? null :
doubles[i]);
+ }
}
- }
- break;
- case STRING:
- Object[] objects = objectSelector.getObjectVector();
- for (int i = 0; i < objectSelector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(objects[i]);
- }
- break;
- }
+ break;
+ case STRING:
+ Object[] objects = objectSelector.getObjectVector();
Review Comment:
## Dereferenced variable may be null
Variable [objectSelector](1) may be null at this access because of [this](2)
assignment.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7607)
##########
processing/src/test/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java:
##########
@@ -433,48 +431,42 @@
final StorageAdapter sa = new IncrementalIndexStorageAdapter(index);
- Sequence<Cursor> cursors = sa.makeCursors(
- null,
- Intervals.utc(timestamp - 60_000, timestamp + 60_000),
- VirtualColumns.EMPTY,
- Granularities.ALL,
- false,
- null
- );
- final AtomicInteger assertCursorsNotEmpty = new AtomicInteger(0);
-
- cursors
- .map(cursor -> {
- DimensionSelector dimSelector = cursor
- .getColumnSelectorFactory()
- .makeDimensionSelector(new DefaultDimensionSpec("billy",
"billy"));
- int cardinality = dimSelector.getValueCardinality();
-
- //index gets more rows at this point, while other thread is
iterating over the cursor
- try {
- for (int i = 0; i < 1; i++) {
- index.add(new MapBasedInputRow(timestamp,
Collections.singletonList("billy"), ImmutableMap.of("billy", "v2" + i)));
- }
- }
- catch (Exception ex) {
- throw new RuntimeException(ex);
- }
-
- int rowNumInCursor = 0;
- // and then, cursoring continues in the other thread
- while (!cursor.isDone()) {
- IndexedInts row = dimSelector.getRow();
- row.forEach(i -> Assert.assertTrue(i < cardinality));
- cursor.advance();
- rowNumInCursor++;
- }
- Assert.assertEquals(2, rowNumInCursor);
- assertCursorsNotEmpty.incrementAndGet();
-
- return null;
- })
- .toList();
- Assert.assertEquals(1, assertCursorsNotEmpty.get());
+ final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
+
.setInterval(Intervals.utc(timestamp - 60_000, timestamp + 60_000))
+
.setGranularity(Granularities.ALL)
+ .build();
+ try (final CursorMaker maker = sa.asCursorMaker(buildSpec)) {
+ Cursor cursor = maker.makeCursor();
+ final AtomicInteger assertCursorsNotEmpty = new AtomicInteger(0);
Review Comment:
## Unread local variable
Variable 'AtomicInteger assertCursorsNotEmpty' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7614)
##########
processing/src/test/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java:
##########
@@ -497,37 +489,28 @@
final StorageAdapter sa = new IncrementalIndexStorageAdapter(index);
- Sequence<Cursor> cursors = sa.makeCursors(
- new DictionaryRaceTestFilter(index, timestamp),
- Intervals.utc(timestamp - 60_000, timestamp + 60_000),
- VirtualColumns.EMPTY,
- Granularities.ALL,
- false,
- null
- );
- final AtomicInteger assertCursorsNotEmpty = new AtomicInteger(0);
-
- cursors
- .map(cursor -> {
- DimensionSelector dimSelector = cursor
- .getColumnSelectorFactory()
- .makeDimensionSelector(new DefaultDimensionSpec("billy",
"billy"));
- int cardinality = dimSelector.getValueCardinality();
-
- int rowNumInCursor = 0;
- while (!cursor.isDone()) {
- IndexedInts row = dimSelector.getRow();
- row.forEach(i -> Assert.assertTrue(i < cardinality));
- cursor.advance();
- rowNumInCursor++;
- }
- Assert.assertEquals(5, rowNumInCursor);
- assertCursorsNotEmpty.incrementAndGet();
-
- return null;
- })
- .toList();
- Assert.assertEquals(1, assertCursorsNotEmpty.get());
+ final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
+ .setFilter(new
DictionaryRaceTestFilter(index, timestamp))
+
.setInterval(Intervals.utc(timestamp - 60_000, timestamp + 60_000))
+
.setGranularity(Granularities.ALL)
+ .build();
+ try (final CursorMaker maker = sa.asCursorMaker(buildSpec)) {
+ Cursor cursor = maker.makeCursor();
+ final AtomicInteger assertCursorsNotEmpty = new AtomicInteger(0);
Review Comment:
## Unread local variable
Variable 'AtomicInteger assertCursorsNotEmpty' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7615)
##########
processing/src/test/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java:
##########
@@ -548,93 +531,88 @@
final StorageAdapter sa = new IncrementalIndexStorageAdapter(index);
- Sequence<Cursor> cursors = sa.makeCursors(
- null,
- Intervals.utc(timestamp - 60_000, timestamp + 60_000),
- VirtualColumns.EMPTY,
- Granularities.ALL,
- false,
- null
- );
- final AtomicInteger assertCursorsNotEmpty = new AtomicInteger(0);
-
- cursors
- .map(cursor -> {
- DimensionSelector dimSelector1A = cursor
- .getColumnSelectorFactory()
- .makeDimensionSelector(new DefaultDimensionSpec("billy",
"billy"));
- int cardinalityA = dimSelector1A.getValueCardinality();
-
- //index gets more rows at this point, while other thread is
iterating over the cursor
- try {
- index.add(new MapBasedInputRow(timestamp,
Collections.singletonList("billy"), ImmutableMap.of("billy", "v1")));
- }
- catch (Exception ex) {
- throw new RuntimeException(ex);
- }
-
- DimensionSelector dimSelector1B = cursor
- .getColumnSelectorFactory()
- .makeDimensionSelector(new DefaultDimensionSpec("billy",
"billy"));
- //index gets more rows at this point, while other thread is
iterating over the cursor
- try {
- index.add(new MapBasedInputRow(timestamp,
Collections.singletonList("billy"), ImmutableMap.of("billy", "v2")));
- index.add(new MapBasedInputRow(timestamp,
Collections.singletonList("billy2"), ImmutableMap.of("billy2", "v3")));
- }
- catch (Exception ex) {
- throw new RuntimeException(ex);
- }
-
- DimensionSelector dimSelector1C = cursor
- .getColumnSelectorFactory()
- .makeDimensionSelector(new DefaultDimensionSpec("billy",
"billy"));
-
- DimensionSelector dimSelector2D = cursor
- .getColumnSelectorFactory()
- .makeDimensionSelector(new DefaultDimensionSpec("billy2",
"billy2"));
- //index gets more rows at this point, while other thread is
iterating over the cursor
- try {
- index.add(new MapBasedInputRow(timestamp,
Collections.singletonList("billy"), ImmutableMap.of("billy", "v3")));
- index.add(new MapBasedInputRow(timestamp,
Collections.singletonList("billy3"), ImmutableMap.of("billy3", "")));
- }
- catch (Exception ex) {
- throw new RuntimeException(ex);
- }
-
- DimensionSelector dimSelector3E = cursor
- .getColumnSelectorFactory()
- .makeDimensionSelector(new DefaultDimensionSpec("billy3",
"billy3"));
-
- int rowNumInCursor = 0;
- // and then, cursoring continues in the other thread
- while (!cursor.isDone()) {
- IndexedInts rowA = dimSelector1A.getRow();
- rowA.forEach(i -> Assert.assertTrue(i < cardinalityA));
- IndexedInts rowB = dimSelector1B.getRow();
- rowB.forEach(i -> Assert.assertTrue(i < cardinalityA));
- IndexedInts rowC = dimSelector1C.getRow();
- rowC.forEach(i -> Assert.assertTrue(i < cardinalityA));
- IndexedInts rowD = dimSelector2D.getRow();
- // no null id, so should get empty dims array
- Assert.assertEquals(0, rowD.size());
- IndexedInts rowE = dimSelector3E.getRow();
- if (NullHandling.replaceWithDefault()) {
- Assert.assertEquals(1, rowE.size());
- // the null id
- Assert.assertEquals(0, rowE.get(0));
- } else {
- Assert.assertEquals(0, rowE.size());
- }
- cursor.advance();
- rowNumInCursor++;
- }
- Assert.assertEquals(2, rowNumInCursor);
- assertCursorsNotEmpty.incrementAndGet();
-
- return null;
- })
- .toList();
- Assert.assertEquals(1, assertCursorsNotEmpty.get());
+ final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
+
.setInterval(Intervals.utc(timestamp - 60_000, timestamp + 60_000))
+
.setGranularity(Granularities.ALL)
+ .build();
+ try (final CursorMaker maker = sa.asCursorMaker(buildSpec)) {
+ Cursor cursor = maker.makeCursor();
+ final AtomicInteger assertCursorsNotEmpty = new AtomicInteger(0);
Review Comment:
## Unread local variable
Variable 'AtomicInteger assertCursorsNotEmpty' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/7616)
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/scan/ScanQueryFrameProcessor.java:
##########
@@ -252,21 +245,23 @@
if (cursor == null) {
final ResourceHolder<Segment> segmentHolder =
closer.register(segment.getOrLoad());
- final Yielder<Cursor> cursorYielder = Yielders.each(
- makeCursors(
- query.withQuerySegmentSpec(new
SpecificSegmentSpec(segment.getDescriptor())),
- mapSegment(segmentHolder.get()).asStorageAdapter()
- )
- );
+ final StorageAdapter adapter =
mapSegment(segmentHolder.get()).asStorageAdapter();
+ if (adapter == null) {
+ throw new ISE(
+ "Null storage adapter found. Probably trying to issue a query
against a segment being memory unmapped."
+ );
+ }
- if (cursorYielder.isDone()) {
+ final CursorMaker maker =
closer.register(adapter.asCursorMaker(query.asCursorBuildSpec(null)));
+ final Cursor cursor = maker.makeCursor();
Review Comment:
## Possible confusion of local and field
Confusing name: method [runWithSegment](1) also refers to field [cursor](2)
(without qualifying it with 'this').
[Show more
details](https://github.com/apache/druid/security/code-scanning/7617)
##########
processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVectorSelectorsTest.java:
##########
@@ -234,109 +235,100 @@
)
);
final QueryableIndexStorageAdapter storageAdapter = new
QueryableIndexStorageAdapter(index);
- VectorCursor cursor = storageAdapter.makeVectorCursor(
- null,
- index.getDataInterval(),
- virtualColumns,
- false,
- 512,
- null
- );
-
- ColumnCapabilities capabilities =
virtualColumns.getColumnCapabilitiesWithFallback(storageAdapter, "v");
-
- int rowCount = 0;
- if (capabilities.isDictionaryEncoded().isTrue()) {
- SingleValueDimensionVectorSelector selector =
cursor.getColumnSelectorFactory().makeSingleValueDimensionSelector(
- DefaultDimensionSpec.of("v")
- );
- while (!cursor.isDone()) {
- int[] row = selector.getRowVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++, rowCount++) {
- results.add(selector.lookupName(row[i]));
+ final CursorBuildSpec buildSpec = CursorBuildSpec.builder()
+
.setInterval(index.getDataInterval())
+
.setGranularity(Granularities.ALL)
+
.setVirtualColumns(virtualColumns)
+ .build();
+ try (final CursorMaker maker = storageAdapter.asCursorMaker(buildSpec)) {
+ final VectorCursor cursor = maker.makeVectorCursor();
+
+ ColumnCapabilities capabilities =
virtualColumns.getColumnCapabilitiesWithFallback(storageAdapter, "v");
+
+ int rowCount = 0;
+ if (capabilities.isDictionaryEncoded().isTrue()) {
+ SingleValueDimensionVectorSelector selector =
cursor.getColumnSelectorFactory()
+
.makeSingleValueDimensionSelector(
+
DefaultDimensionSpec.of("v")
+ );
+ while (!cursor.isDone()) {
+ int[] row = selector.getRowVector();
+ for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
+ results.add(selector.lookupName(row[i]));
+ }
+ cursor.advance();
}
- cursor.advance();
- }
- } else {
- VectorValueSelector selector = null;
- VectorObjectSelector objectSelector = null;
- if (Types.isNumeric(outputType)) {
- selector = cursor.getColumnSelectorFactory().makeValueSelector("v");
} else {
- objectSelector =
cursor.getColumnSelectorFactory().makeObjectSelector("v");
- }
- GroupByVectorColumnSelector groupBySelector =
-
cursor.getColumnSelectorFactory().makeGroupByVectorColumnSelector("v",
DeferExpressionDimensions.ALWAYS);
- while (!cursor.isDone()) {
- final List<Object> resultsVector = new ArrayList<>();
- boolean[] nulls;
- switch (outputType.getType()) {
- case LONG:
- nulls = selector.getNullVector();
- long[] longs = selector.getLongVector();
- for (int i = 0; i < selector.getCurrentVectorSize(); i++,
rowCount++) {
- resultsVector.add(nulls != null && nulls[i] ? null : longs[i]);
- }
- break;
- case DOUBLE:
- // special case to test floats just to get coverage on
getFloatVector
- if ("float2".equals(expression)) {
+ VectorValueSelector selector = null;
+ VectorObjectSelector objectSelector = null;
+ if (Types.isNumeric(outputType)) {
+ selector = cursor.getColumnSelectorFactory().makeValueSelector("v");
+ } else {
+ objectSelector =
cursor.getColumnSelectorFactory().makeObjectSelector("v");
+ }
+ GroupByVectorColumnSelector groupBySelector =
+
cursor.getColumnSelectorFactory().makeGroupByVectorColumnSelector("v",
DeferExpressionDimensions.ALWAYS);
+ while (!cursor.isDone()) {
+ final List<Object> resultsVector = new ArrayList<>();
+ boolean[] nulls;
+ switch (outputType.getType()) {
Review Comment:
## Missing enum case in switch
Switch statement does not have a case for [ARRAY](1) or [COMPLEX](2).
[Show more
details](https://github.com/apache/druid/security/code-scanning/7619)
##########
extensions-core/multi-stage-query/src/main/java/org/apache/druid/msq/querykit/scan/ScanQueryFrameProcessor.java:
##########
@@ -294,15 +289,22 @@
final Frame frame = inputChannel.read();
final FrameSegment frameSegment = new FrameSegment(frame,
inputFrameReader, SegmentId.dummy("scan"));
- final long rowsFlushed = setNextCursor(
- Iterables.getOnlyElement(
- makeCursors(
- query.withQuerySegmentSpec(new
MultipleIntervalSegmentSpec(Intervals.ONLY_ETERNITY)),
- mapSegment(frameSegment).asStorageAdapter()
- ).toList()
- ),
- frameSegment
- );
+ final StorageAdapter adapter =
mapSegment(frameSegment).asStorageAdapter();
+ if (adapter == null) {
+ throw new ISE(
+ "Null storage adapter found. Probably trying to issue a query
against a segment being memory unmapped."
+ );
+ }
+
+ final CursorMaker maker =
closer.register(adapter.asCursorMaker(query.asCursorBuildSpec(null)));
+ final Cursor cursor = maker.makeCursor();
Review Comment:
## Possible confusion of local and field
Confusing name: method [runWithInputChannel](1) also refers to field
[cursor](2) (without qualifying it with 'this').
[Show more
details](https://github.com/apache/druid/security/code-scanning/7618)
--
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]
