Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESQL: Method to convert BooleanBlock to a "mask" #112253

Merged
merged 5 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ public OrdinalBytesRefBlock asOrdinals() {
return null;
}

@Override
public ToMask toMask() {
return new ToMask(blockFactory.newConstantBooleanVector(false, positionCount), false);
}

@Override
public boolean isNull(int position) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ public OrdinalBytesRefVector asOrdinals() {
throw new UnsupportedOperationException("null vector");
}

@Override
public ToMask toMask() {
assert false : "null vector";
throw new UnsupportedOperationException("null vector");
}

@Override
public ConstantNullVector filter(int... positions) {
assert false : "null vector";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.compute.data;

import org.elasticsearch.core.Releasable;

/**
* Result from calling {@link BooleanBlock#toMask}. {@link #close closing} this will
* close the contained {@link #mask()}. If you want to keep a reference to it then you'll
* have to {@link Block#incRef()} it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this doesn't implement incref/RefCounted/AbstractNonThreadSafeRefCounted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Block's ref counted - this is sort of like a single reference to it. Not sure if that's clear though.

*/
public record ToMask(BooleanVector mask, boolean hadMultivaluedFields) implements Releasable {
alex-spies marked this conversation as resolved.
Show resolved Hide resolved
@Override
public void close() {
mask.close();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,32 @@ $if(BytesRef)$
public OrdinalBytesRefBlock asOrdinals() {
return null;
}

$elseif(boolean)$
/**
* Convert this to a {@link BooleanVector "mask"} that's appropriate for
* passing to {@link #keepMask}.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: let's describe what to expect in case of null values and multivalues.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

*/
@Override
public ToMask toMask() {
if (getPositionCount() == 0) {
return new ToMask(blockFactory().newConstantBooleanVector(false, 0), false);
}
try (BooleanVector.FixedBuilder builder = blockFactory().newBooleanVectorFixedBuilder(getPositionCount())) {
boolean hasMv = false;
for (int p = 0; p < getPositionCount(); p++) {
builder.appendBoolean(switch (getValueCount(p)) {
case 0 -> false;
case 1 -> getBoolean(getFirstValueIndex(p));
default -> {
hasMv = true;
yield false;
}
});
}
return new ToMask(builder.build(), hasMv);
}
}
$endif$

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,19 @@ $if(BytesRef)$
public OrdinalBytesRefVector asOrdinals() {
return null;
}
$endif$

$elseif(boolean)$
/**
* Convert this to a {@link BooleanVector "mask"} that's appropriate for
* passing to {@link #keepMask}.
*/
@Override
public ToMask toMask() {
incRef();
return new ToMask(this, false);
}

$endif$
$if(BytesRef)$
@Override
public BytesRef getBytesRef(int position, BytesRef dest) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,33 @@ public final class $Type$BigArrayBlock extends AbstractArrayBlock implements $Ty
return null;
}

$if(boolean)$
/**
* Convert this to a {@link BooleanVector "mask"} that's appropriate for
* passing to {@link #keepMask}.
*/
@Override
public ToMask toMask() {
if (getPositionCount() == 0) {
return new ToMask(blockFactory().newConstantBooleanVector(false, 0), false);
}
try (BooleanVector.FixedBuilder builder = blockFactory().newBooleanVectorFixedBuilder(getPositionCount())) {
boolean hasMv = false;
for (int p = 0; p < getPositionCount(); p++) {
builder.appendBoolean(switch (getValueCount(p)) {
case 0 -> false;
case 1 -> getBoolean(getFirstValueIndex(p));
default -> {
hasMv = true;
yield false;
}
});
}
return new ToMask(builder.build(), hasMv);
}
}
$endif$

@Override
public $type$ get$Type$(int valueIndex) {
return vector.get$Type$(valueIndex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,18 @@ $endif$
return new $Type$VectorBlock(this);
}

$if(boolean)$
/**
* Convert this to a {@link BooleanVector "mask"} that's appropriate for
* passing to {@link #keepMask}.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: This is duplicated in most of the templates, but having this javadoc on the boolean block/vector interfaces should suffice. (Just tested this for ConstantNullBlock, which doesn't have this javadoc comment, and when hovering over toMask I get the correct javadoc from the interface.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed that bit.

@Override
public ToMask toMask() {
incRef();
return new ToMask(this, false);
}
$endif$

@Override
public $type$ get$Type$(int position) {
return values.get(position);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,15 @@ $if(BytesRef)$
* returns null. Callers must not release the returned block as no extra reference is retained by this method.
*/
OrdinalBytesRefBlock asOrdinals();
$endif$

$elseif(boolean)$
/**
* Convert this to a {@link BooleanVector "mask"} that's appropriate for
* passing to {@link #keepMask}.
*/
ToMask toMask();

$endif$
@Override
$Type$Block filter(int... positions);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,19 @@ $if(BytesRef)$
public OrdinalBytesRefVector asOrdinals() {
return null;
}
$endif$

$elseif(boolean)$
/**
* Convert this to a {@link BooleanVector "mask"} that's appropriate for
* passing to {@link #keepMask}.
*/
@Override
public ToMask toMask() {
incRef();
return new ToMask(this, false);
}

$endif$
@Override
public $Type$Vector filter(int... positions) {
return blockFactory().newConstant$Type$Vector(value, positions.length);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,15 @@ $if(BytesRef)$
* returns null. Callers must not release the returned vector as no extra reference is retained by this method.
*/
OrdinalBytesRefVector asOrdinals();
$endif$

$elseif(boolean)$
/**
* Convert this to a {@link BooleanVector "mask"} that's appropriate for
* passing to {@link #keepMask}.
*/
ToMask toMask();

$endif$
@Override
$Type$Vector filter(int... positions);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,18 @@ $if(BytesRef)$
return null;
}
}
$endif$

$elseif(boolean)$
/**
* Convert this to a {@link BooleanVector "mask"} that's appropriate for
* passing to {@link #keepMask}.
*/
@Override
public ToMask toMask() {
return vector.toMask();
}

$endif$
@Override
$if(BytesRef)$
public BytesRef getBytesRef(int valueIndex, BytesRef dest) {
Expand Down
Loading