-
Notifications
You must be signed in to change notification settings - Fork 18
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
Expressions #74
Expressions #74
Conversation
# Conflicts: # src/main/java/com/yahoo/bullet/postaggregations/ComputationStrategy.java
ready for re-review. i've undoubtedly missed something |
src/main/java/com/yahoo/bullet/query/postaggregations/Culling.java
Outdated
Show resolved
Hide resolved
src/main/java/com/yahoo/bullet/query/postaggregations/Computation.java
Outdated
Show resolved
Hide resolved
src/main/java/com/yahoo/bullet/query/postaggregations/Having.java
Outdated
Show resolved
Hide resolved
src/main/java/com/yahoo/bullet/query/postaggregations/OrderBy.java
Outdated
Show resolved
Hide resolved
src/main/java/com/yahoo/bullet/querying/aggregations/QuantileSketchingStrategy.java
Outdated
Show resolved
Hide resolved
return null; | ||
} | ||
|
||
private static double[] generatePoints(double start, double end, double increment, int maxPoints, int rounding) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Share this with the code in QuantileSketch? Perhaps in utilities and make the signature have a function instead of the increment so we can generalize to logarithmic, exponential and whatever else point generation?
src/main/java/com/yahoo/bullet/querying/aggregations/grouping/GroupData.java
Outdated
Show resolved
Hide resolved
TypedObject leftValue = left.evaluate(record); | ||
TypedObject rightValue = right.evaluate(record); | ||
Type subType = leftValue.getType(); | ||
return new TypedObject(Type.BOOLEAN, ((List<? extends Serializable>) rightValue.getValue()).stream().anyMatch(o -> leftValue.compareTo(new TypedObject(subType, o)) > 0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't remember but do these work properly for vacuous truth checks? X > ANY([]) or X > ALL([])? I remember having to do stuff to handle this properly.
@@ -90,6 +90,6 @@ public void testNumericExtraction() { | |||
Assert.assertNull(Utilities.extractFieldAsNumber("id", record)); | |||
Assert.assertEquals(Utilities.extractFieldAsNumber("foo", record), ((Number) 1.20).doubleValue()); | |||
Assert.assertEquals(Utilities.extractFieldAsNumber("bar", record), ((Number) 42).longValue()); | |||
Assert.assertEquals(Utilities.extractFieldAsNumber("map_field.foo", record), ((Number) 21).doubleValue()); | |||
Assert.assertNull(Utilities.extractFieldAsNumber("map_field.foo", record)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just remove the addMap instead since this is functionality we no longer support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I kept this test here to show that that functionality doesn't exist anymore.
src/test/java/com/yahoo/bullet/windowing/SlidingRecordTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/yahoo/bullet/storage/MemoryStorageManagerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/yahoo/bullet/querying/aggregations/GroupStrategyTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/yahoo/bullet/querying/aggregations/QuantileSketchingStrategyTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/yahoo/bullet/querying/aggregations/TupleSketchingStrategyTest.java
Outdated
Show resolved
Hide resolved
Assert.assertEquals(GroupData.getResultName(operation), GroupOperation.GroupOperationType.COUNT.getName() + | ||
GroupData.NAME_SEPARATOR + "foo"); | ||
operation = new GroupOperation(GroupOperation.GroupOperationType.COUNT, null, "bar"); | ||
Assert.assertEquals(GroupData.getResultName(operation), "bar"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test for the new fieldAliases added to GroupData?
src/test/java/com/yahoo/bullet/querying/partitioning/SimpleEqualityPartitionerTest.java
Outdated
Show resolved
Hide resolved
return new TypedObject(record.extractField(identifier)); | ||
public static double[] generatePoints(double start, Function<Double, Double> generator, int numberOfPoints, int rounding) { | ||
double[] points = new double[numberOfPoints]; | ||
double begin = start; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call it value?
@Slf4j | ||
public class GroupAll implements Strategy { | ||
public class GroupStrategy implements Strategy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GroupAllStrategy?
No description provided.