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

feat: implement AggCountWhere to support generic counting using Filter #6497

Merged
merged 21 commits into from
Dec 19, 2024

Conversation

lbooker42
Copy link
Contributor

@lbooker42 lbooker42 commented Dec 17, 2024

Groovy Examples

Table countTable = table.aggBy(List.of(
        AggCountWhere("filter1", "intCol >= 50"),
        AggCountWhere("filter2", "intCol >= 50", "intCol != 80"),
        AggCountWhere("filter3", Filter.or(Filter.from("intCol >= 50", "intCol == 3"))),
        AggCountWhere("filter4", "true"),
        AggCountWhere("filter5", "false"),
        AggCountWhere("filter6", "intCol % 2 == 0"),
        AggCountWhere("filter7",
                Filter.and(Filter.or(Filter.from("false", "intCol % 3 == 0")),
                        Filter.or(Filter.from("false", "intCol % 2 == 0")))),
        AggCountWhere("filter8", "intCol % 2 == 0", "intCol % 3 == 0"),
        AggCountWhere("filter9",
                Filter.and(Filter.and(Filter.from("intCol > 0")),
                        Filter.and(Filter.from("intCol <= 10", "intCol >= 5")))),
        // Multiple input columns
        AggCountWhere("filter10", "intCol >= 5", "doubleCol <= 10.0"),
        AggCountWhere("filter11", "intCol >= 5 && intColNulls != 3 && doubleCol <= 10.0"),
        // DynamicWhereFilter
        AggCountWhere("filter12", new DynamicWhereFilter(setTable, true, MatchPairFactory.getExpressions("intCol")))),
        "Sym").sort("Sym");

Python Examples

from deephaven import empty_table
from deephaven.agg import count_where

table_size = 120000000
table = empty_table(table_size).update(["int1M=randomInt(0,1000000)", "int640=randomInt(0,640)", "int250=randomInt(0,250)"]).select()

# zero-key
t_count = table.agg_by(aggs=count_where(col="count", filters=["int1M < 500000", "int1M > 499000"]))

# bucketed
t_count = table.agg_by(aggs=count_where(col="count", filters=["int1M < 500000", "int1M > 499000"]), by="int250")

# Conflicts:
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/ByteChunkMatchFilterFactory.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/ByteRangeComparator.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/CharChunkMatchFilterFactory.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/CharRangeComparator.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/DoubleChunkMatchFilterFactory.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/FloatChunkMatchFilterFactory.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/IntChunkMatchFilterFactory.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/IntRangeComparator.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/LongChunkMatchFilterFactory.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/ShortChunkMatchFilterFactory.java
#	engine/table/src/main/java/io/deephaven/engine/table/impl/chunkfilter/ShortRangeComparator.java
Copy link
Contributor

@jmao-denver jmao-denver left a comment

Choose a reason for hiding this comment

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

Python changes mostly look good with a questionable indentation change.

py/server/deephaven/agg.py Outdated Show resolved Hide resolved
Copy link
Contributor

@cpwright cpwright left a comment

Choose a reason for hiding this comment

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

The filter changes look good to me, and I'm happy that the code size only modestly increased this way.

…ient tests for verification of API and GRPC implementations.
cpwright
cpwright previously approved these changes Dec 18, 2024
Copy link
Contributor

@cpwright cpwright left a comment

Choose a reason for hiding this comment

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

Want to check coverage before we merge, but I think all my concerns are addressed.

cpwright
cpwright previously approved these changes Dec 18, 2024
@lbooker42 lbooker42 merged commit 83e0c97 into deephaven:main Dec 19, 2024
17 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2024
@deephaven-internal
Copy link
Contributor

Labels indicate documentation is required. Issues for documentation have been opened:

Community: deephaven/deephaven-docs-community#369

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants