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

chore(sql): fix occasional failures with first()/last()/first_not_null()/last_not_null() direct string function #4324

Merged
merged 15 commits into from Mar 22, 2024

Conversation

jerrinot
Copy link
Contributor

@jerrinot jerrinot commented Mar 21, 2024

Continuation of #4310

the isDirectStr() flags are gone. I created specialized sink-like holder which supports both direct and non-directs char sequences. it's not a sink since it does not support appending to previously set string. it always discard any previously held string.

I'm pushing into @puzpuzpuz's branch since it's building on top of his changes and I want him to give him a chance to review it instead of doing a direct push.

since not all DirectString are stable during a query execution
@puzpuzpuz puzpuzpuz changed the base branch from puzpuzpuz_fix_first_last_direct_str_functions to master March 22, 2024 10:19
@puzpuzpuz puzpuzpuz added the SQL Issues or changes relating to SQL execution label Mar 22, 2024
@puzpuzpuz puzpuzpuz changed the title chore(sql): no more direct str flags chore(sql): fix first/last/first_not_null/last_not_null direct string function usage Mar 22, 2024
@puzpuzpuz puzpuzpuz added the Bug Incorrect or unexpected behavior label Mar 22, 2024
@jerrinot jerrinot marked this pull request as draft March 22, 2024 11:15
@jerrinot jerrinot marked this pull request as ready for review March 22, 2024 13:18
puzpuzpuz
puzpuzpuz previously approved these changes Mar 22, 2024
Copy link
Contributor

@puzpuzpuz puzpuzpuz left a comment

Choose a reason for hiding this comment

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

The approach is better than the boolean flag one. Thanks for experimenting with it!

@puzpuzpuz puzpuzpuz self-requested a review March 22, 2024 15:17
@jerrinot jerrinot changed the title chore(sql): fix first/last/first_not_null/last_not_null direct string function usage chore(sql): fix occasional failures with first()/last()/first_not_null()/last_not_null() direct string function Mar 22, 2024
@bluestreak01 bluestreak01 merged commit 2fd5292 into master Mar 22, 2024
4 of 6 checks passed
@bluestreak01 bluestreak01 deleted the jh_directstr_flags_be_gone branch March 22, 2024 15:46
@ideoma
Copy link
Collaborator

ideoma commented Mar 22, 2024

[PR Coverage check]

😍 pass : 453 / 569 (79.61%)

file detail

path covered line new line coverage
🔵 io/questdb/griffin/engine/functions/groupby/StringAggVarcharGroupByFunction.java 0 8 00.00%
🔵 io/questdb/griffin/engine/functions/test/TestSumTDoubleGroupByFunction.java 0 2 00.00%
🔵 io/questdb/griffin/engine/functions/test/TestSumDoubleGroupByFunction.java 0 2 00.00%
🔵 io/questdb/griffin/engine/functions/test/TestSumStringGroupByFunction.java 0 2 00.00%
🔵 io/questdb/griffin/engine/functions/groupby/IsIPv4OrderedGroupByFunction.java 0 2 00.00%
🔵 io/questdb/griffin/engine/functions/groupby/IsLongOrderedGroupByFunction.java 0 2 00.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstVarcharGroupByFunction.java 0 9 00.00%
🔵 io/questdb/griffin/engine/functions/groupby/CountDistinctVarcharGroupByFunction.java 0 6 00.00%
🔵 io/questdb/griffin/engine/functions/groupby/InterpolationGroupByFunction.java 0 7 00.00%
🔵 io/questdb/griffin/engine/functions/groupby/StringAggGroupByFunction.java 1 4 25.00%
🔵 io/questdb/griffin/engine/functions/groupby/SumLong256GroupByFunction.java 1 4 25.00%
🔵 io/questdb/griffin/engine/functions/groupby/AbstractCovarGroupByFunction.java 1 2 50.00%
🔵 io/questdb/griffin/engine/functions/groupby/KSumDoubleGroupByFunction.java 1 2 50.00%
🔵 io/questdb/griffin/engine/functions/groupby/CorrGroupByFunction.java 1 2 50.00%
🔵 io/questdb/griffin/engine/functions/groupby/NSumDoubleGroupByFunction.java 1 2 50.00%
🔵 io/questdb/griffin/engine/functions/groupby/AbstractStdDevGroupByFunction.java 1 2 50.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinVarcharGroupByFunction.java 8 14 57.14%
🔵 io/questdb/griffin/engine/functions/groupby/MaxDoubleGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/ApproxPercentileLongPackedGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinTimestampGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MaxFloatGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/ApproxPercentileDoublePackedGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinFloatGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MaxCharGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinIPv4GroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinDoubleGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MaxLongGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinDateGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/ApproxPercentileDoubleGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MaxIPv4GroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MaxTimestampGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MaxDateGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinLongGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinIntGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinCharGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/ApproxPercentileLongGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/CountLongConstGroupByFunction.java 3 5 60.00%
🔵 io/questdb/griffin/engine/functions/groupby/SumLongGroupByFunction.java 4 6 66.67%
🔵 io/questdb/griffin/engine/functions/groupby/CountDistinctStringGroupByFunction.java 4 6 66.67%
🔵 io/questdb/griffin/engine/functions/groupby/CountDistinctSymbolGroupByFunction.java 4 6 66.67%
🔵 io/questdb/griffin/engine/functions/groupby/SumDoubleGroupByFunction.java 4 6 66.67%
🔵 io/questdb/griffin/engine/functions/groupby/SumFloatGroupByFunction.java 4 6 66.67%
🔵 io/questdb/griffin/engine/functions/groupby/VwapDoubleGroupByFunction.java 5 7 71.43%
🔵 io/questdb/griffin/engine/functions/groupby/HaversineDistDegreeGroupByFunction.java 9 11 81.82%
🔵 io/questdb/griffin/SqlCodeGenerator.java 60 63 95.24%
🔵 io/questdb/griffin/engine/groupby/StableAwareStringHolder.java 50 52 96.15%
🔵 io/questdb/griffin/engine/functions/groupby/CountDistinctUuidGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/LastStrGroupByFunctionFactory.java 1 1 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstIntGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstShortGroupByFunction.java 6 6 100.00%
🔵 io/questdb/std/str/StableDirectString.java 1 1 100.00%
🔵 io/questdb/cairo/TableReader.java 1 1 100.00%
🔵 io/questdb/cairo/sql/PageAddressCacheRecord.java 2 2 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstFloatGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstCharGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstDoubleGroupByFunction.java 6 6 100.00%
🔵 io/questdb/cairo/vm/Vm.java 2 2 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/MaxVarcharGroupByFunction.java 5 5 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/LastNotNullStrGroupByFunction.java 1 1 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/LastStrGroupByFunction.java 1 1 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstStrGroupByFunctionFactory.java 1 1 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstNotNullStrGroupByFunctionFactory.java 1 1 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstSymbolGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstGeoHashGroupByFunctionLong.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/ApproxCountDistinctIntGroupByFunction.java 9 9 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/ApproxCountDistinctIPv4GroupByFunction.java 9 9 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/AvgDoubleGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstBooleanGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstIPv4GroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/AbstractCountGroupByFunction.java 5 5 100.00%
🔵 io/questdb/cairo/vm/MemoryFCRImpl.java 2 2 100.00%
🔵 io/questdb/griffin/engine/groupby/GroupByUtils.java 2 2 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/CountDistinctIPv4GroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstLongGroupByFunction.java 6 6 100.00%
🔵 io/questdb/cairo/vm/MemoryCMRImpl.java 2 2 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstTimestampGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/MaxStrGroupByFunction.java 5 5 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/CountDistinctLongGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/ApproxCountDistinctLongGroupByFunction.java 9 9 100.00%
🔵 io/questdb/cairo/vm/MemoryCARWImpl.java 1 1 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/MinStrGroupByFunction.java 5 5 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/LastNotNullStrGroupByFunctionFactory.java 1 1 100.00%
🔵 io/questdb/cairo/vm/MemoryCMARWImpl.java 2 2 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstByteGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/SumIntGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstGeoHashGroupByFunctionByte.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstStrGroupByFunction.java 9 9 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/CountDistinctLong256GroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstNotNullStrGroupByFunction.java 1 1 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstGeoHashGroupByFunctionShort.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstDateGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/MaxIntGroupByFunction.java 5 5 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstGeoHashGroupByFunctionInt.java 6 6 100.00%
🔵 io/questdb/cairo/vm/AbstractMemoryCR.java 7 7 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/CountDistinctIntGroupByFunction.java 6 6 100.00%
🔵 io/questdb/griffin/engine/functions/groupby/FirstUuidGroupByFunction.java 6 6 100.00%

terasum pushed a commit to terasum/questdb that referenced this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Incorrect or unexpected behavior SQL Issues or changes relating to SQL execution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants