Skip to content

sampler returns nulls in CSV#8871

Merged
jon-wei merged 6 commits intoapache:masterfrom
SEKIRO-J:nullSwallow
Nov 19, 2019
Merged

sampler returns nulls in CSV#8871
jon-wei merged 6 commits intoapache:masterfrom
SEKIRO-J:nullSwallow

Conversation

@SEKIRO-J
Copy link
Copy Markdown
Contributor

@SEKIRO-J SEKIRO-J commented Nov 14, 2019

Fixes #8845.

Description

When parsing a CSV in the sampler, if the whole column values are not provided but column header is provided, this column will be ignored.

Ideally we still want to return this column with values of null or "".
In this implementation it will return ""


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths.
  • added integration tests.
  • been tested in a test Druid cluster.

@jihoonson jihoonson added the Bug label Nov 15, 2019
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this should just return nulls as is, any null->empty string conversion should have happened already at this point, so any nulls you see here should really be null

@SEKIRO-J SEKIRO-J force-pushed the nullSwallow branch 2 times, most recently from 4e57bca to dde7325 Compare November 19, 2019 00:07
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Nov 19, 2019

I restarted the failed integration test. I'm not sure if it's transient or not, but we'll see.

@jon-wei jon-wei merged commit d091347 into apache:master Nov 19, 2019
jon-wei pushed a commit to jon-wei/druid that referenced this pull request Nov 26, 2019
* sampler returns nulls in CSV

* fixed kafka sampler test

* fix Kinesis test

* sql compatibility fix

* remove null to empty string conversion, use null

* fix sql compatibility
@jon-wei jon-wei added this to the 0.17.0 milestone Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sampler swallows up nulls in CSV

4 participants