Skip to content

Bucketing fixes#1336

Merged
findepi merged 3 commits intotrinodb:masterfrom
findepi:rebuck
Aug 21, 2019
Merged

Bucketing fixes#1336
findepi merged 3 commits intotrinodb:masterfrom
findepi:rebuck

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Aug 20, 2019

No description provided.

Copy link
Copy Markdown
Member

@kokosing kokosing Aug 20, 2019

Choose a reason for hiding this comment

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

can you add a test that for two different ints that represents NaN in float they are producing same hash? That would be much better than a comment in the code.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'd need a new connector where I can force a particular bits representation of a NaN value...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for catching me. I added a unit test and it told me there's another method to update.

This matches
https://github.com/apache/hive/blob/ba83fd7bff/serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorUtils.java#L554
more closely.

We already do *not* assume canonical on-stack representation for floats
in `io.prestosql.spi.type.RealType#hash`.
Before the change, selecting `"$bucket"` would fail at run-time with

```
Query 20190820_065833_00012_nk9ff failed: No value present
java.util.NoSuchElementException: No value present
	at java.util.OptionalInt.getAsInt(OptionalInt.java:118)
	at io.prestosql.plugin.hive.HiveUtil.getPrefilledColumnValue(HiveUtil.java:920)
	at io.prestosql.plugin.hive.HivePageSourceProvider$ColumnMapping.buildColumnMappings(HivePageSourceProvider.java:332)
	at io.prestosql.plugin.hive.HivePageSourceProvider.createHivePageSource(HivePageSourceProvider.java:143)
	at io.prestosql.plugin.hive.HivePageSourceProvider.createPageSource(HivePageSourceProvider.java:98)
```
@findepi findepi merged commit 91e8bf9 into trinodb:master Aug 21, 2019
@findepi findepi deleted the rebuck branch August 21, 2019 06:29
@findepi findepi mentioned this pull request Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants