SEPURI-SAI-KRISHNA opened a new pull request, #13046:
URL: https://github.com/apache/seatunnel/pull/13046
Yes, glad to contribute to Apache SeaTunnel.
### Purpose of this pull request
Closes #13045.
`DruidWriter.write` left a null field out of the CSV line instead of writing
an empty value. The row came out one field short, every later value moved one
column left, and the `timestamp` column the writer appends ended up empty.
Druid's CSV reader rejects such a row as having no timestamp, and because the
sink submits the task without a `tuningConfig`, Druid's defaults
(`maxParseExceptions = Integer.MAX_VALUE`, `logParseExceptions = false`) drop
it silently while the task and the job succeed.
**The fix.** A null is written as an empty field:
```java
joiner.add(v == null ? "" : v.toString());
```
Rows without nulls are encoded exactly as before. An empty string was
already written as an empty field, so Druid cannot tell an empty string from a
null; that is inherent to the CSV encoding and unchanged here.
### Does this PR introduce any user facing change?
Yes. A row with a null field is now ingested, where before it was silently
dropped. Druid stores the null according to
`druid.generic.useDefaultValueForNull`: with the default `true`, as `''` for
string columns and `0` for numeric columns; with `false`, as a SQL null. The en
and zh sink documentation now says this. No option, default or schema changed.
I did not add an `incompatible-changes.md` entry, because rows that used to
be lost are now written and nothing that was written before changes. I am happy
to add one if a maintainer prefers.
### How was this patch tested?
**Unit tests.** `connector-druid` only had `DruidFactoryTest`, which covers
factory options, so the writer tests go in a new `DruidWriterTest`. Both drive
the real `write()` and `close()` against a local HTTP stand-in for the Druid
Overlord and inspect the indexing task that is submitted:
- `aNullFieldKeepsItsColumn`: rows `(1, "a", 10)`, `(2, null, 20)` and `(3,
"c", null)` are submitted as `1,a,10,<ts>`, `2,,20,<ts>` and `3,c,,<ts>`.
- `druidReadsEveryRowThatHasANullField`: the submitted data is read back
with Druid 24.0.1's own `CsvInputFormat` and the columns the task declares. All
three rows parse, with the null read as null and `amount` still `20`.
Mutation check, with `DruidWriter` reverted to `dev`:
| test | result on `dev` |
| --- | --- |
| `aNullFieldKeepsItsColumn` | fails: `expected: <2,,20,…> but was:
<2,20,…>` |
| `druidReadsEveryRowThatHasANullField` | fails: `ParseException:
Timestamp[null] is unparseable! Event: {id=2, name=20, amount=…}` |
**E2E.** `DruidIT.testDruidSinkWithNullFields` runs the new
`fakesource_to_druid_with_null.conf`, which writes the same three rows to a
datasource of its own, then queries it and asserts that the rows with ids 2 and
3 are present and that `"amount":20}` appears, which shows the value after the
null stayed in its own column. How Druid renders the null itself depends on its
null handling, so that is not asserted. I used a new configuration file rather
than editing `fakesource_to_druid.conf`, which #11307 also changes.
Docker is not available to me, so `DruidIT` has not run locally.
`DruidIT.java` compiles, and the new configuration parses.
**Build checks**, JDK 17:
- `./mvnw test -pl seatunnel-connectors-v2/connector-druid`: 9 tests, 0
failures.
- `./mvnw spotless:check` on `connector-druid` and `connector-druid-e2e`:
clean.
- `./mvnw -B -T 1 test -D"license.skipAddThirdParty"=true -pl
seatunnel-ci-tools`: 34 tests, 0 failures.
### Check list
* [X] Code changed are covered with tests, or it does not need tests for
reason
* [X] If any new Jar binary package adding in your PR, please add License
Notice according to
[License
Notice](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md):
none added.
* [X] If necessary, please update the documentation to describe the new
feature: the en and zh Druid sink pages describe how a null is written.
* [X] If you are contributing the connector code, please check that the
following files are updated:
1. Update
[plugin-mapping.properties](https://github.com/apache/seatunnel/blob/dev/plugin-mapping.properties):
not needed, no new connector.
2. Update the pom file of
[seatunnel-dist](https://github.com/apache/seatunnel/blob/dev/seatunnel-dist/pom.xml):
not needed, no new module.
3. Add ci label in
[label-scope-conf](https://github.com/apache/seatunnel/blob/dev/.github/workflows/labeler/label-scope-conf.yml):
not needed, `connector-druid` is already labelled.
4. Add e2e test for your connector: `DruidIT` is extended.
5. Update [connector-v2
doc](https://github.com/apache/seatunnel/tree/dev/docs/en/connectors): done, en
and zh.
* [X] Update the `release-note`: not applicable, the file was removed from
`dev` in #9304.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
