You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/04/05 10:02:54 UTC
[GitHub] [flink] fapaul commented on a diff in pull request #19302: [FLINK-26810][connectors/elasticsearch] Use local timezone for TIMESTAMP_WITH_LOCAL_TIMEZONE fields in dynamic index
fapaul commented on code in PR #19302:
URL: https://github.com/apache/flink/pull/19302#discussion_r842598408
##########
flink-connectors/flink-connector-elasticsearch-base/src/test/java/org/apache/flink/streaming/connectors/elasticsearch/table/IndexGeneratorFactoryTest.java:
##########
@@ -197,8 +198,17 @@ public void testDynamicIndexDefaultFormatTimestampWithLocalTimeZone() {
IndexGenerator indexGenerator =
IndexGeneratorFactory.createIndexGenerator("my-index-{local_timestamp|}", schema);
indexGenerator.open();
- Assert.assertEquals("my-index-2020_03_18_12_12_14Z", indexGenerator.generate(rows.get(0)));
- Assert.assertEquals("my-index-2020_03_19_12_12_14Z", indexGenerator.generate(rows.get(1)));
+ if (ZoneId.systemDefault().equals(ZoneId.of("UTC"))) {
+ Assert.assertEquals(
+ "my-index-2020_03_18_12_12_14Z", indexGenerator.generate(rows.get(0)));
+ Assert.assertEquals(
+ "my-index-2020_03_19_12_12_14Z", indexGenerator.generate(rows.get(1)));
+ } else {
+ Assert.assertNotEquals(
+ "my-index-2020_03_18_12_12_14Z", indexGenerator.generate(rows.get(0)));
+ Assert.assertNotEquals(
+ "my-index-2020_03_19_12_12_14Z", indexGenerator.generate(rows.get(1)));
Review Comment:
The assertion looks wrong and both condition branch assert the same.
##########
flink-connectors/flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/table/IndexGeneratorTest.java:
##########
@@ -101,9 +105,37 @@
LocalDateTime.of(2020, 3, 19, 12, 22, 14, 1000)),
(int) LocalDate.of(2020, 3, 19).toEpochDay(),
(int) (LocalTime.of(12, 13, 14, 2000).toNanoOfDay() / 1_000_000L),
+ TimestampData.fromTimestamp(Timestamp.valueOf("2020-03-19 12:22:14")),
"test2",
false));
+ @Test
+ public void testDynamicIndexFromTimestampTz() {
+ IndexGenerator indexGenerator =
+ IndexGeneratorFactory.createIndexGenerator(
+ "{local_timestamp|yyyy_MM_dd_HH-ss}_index", fieldNames, dataTypes);
+ indexGenerator.open();
+ String index = indexGenerator.generate(rows.get(0));
+
+ if (ZoneId.systemDefault().equals(ZoneId.of("UTC"))) {
+ Assertions.assertEquals("2020_03_18_12-14_index", index);
+ } else {
+ Assertions.assertNotEquals("2020_03_18_12-14_index", index);
+ }
+
+ IndexGenerator indexGenerator1 =
+ IndexGeneratorFactory.createIndexGenerator(
+ "{local_timestamp|yyyy_MM_dd_HH-ss}_index", fieldNames, dataTypes);
+ indexGenerator1.open();
+ String index1 = indexGenerator.generate(rows.get(0));
+
+ if (ZoneId.systemDefault().equals(ZoneId.of("UTC"))) {
+ Assertions.assertEquals("2020-03-19_12_22-14_index", index1);
+ } else {
+ Assertions.assertNotEquals("2020-03-19_12_22-14_index", index1);
+ }
Review Comment:
The second part of the tests looks very similar to the first part, please extract a common method or better use a parameterized test to ensure both scenarios.
##########
flink-connectors/flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/table/IndexGeneratorTest.java:
##########
@@ -101,9 +105,37 @@
LocalDateTime.of(2020, 3, 19, 12, 22, 14, 1000)),
(int) LocalDate.of(2020, 3, 19).toEpochDay(),
(int) (LocalTime.of(12, 13, 14, 2000).toNanoOfDay() / 1_000_000L),
+ TimestampData.fromTimestamp(Timestamp.valueOf("2020-03-19 12:22:14")),
"test2",
false));
+ @Test
+ public void testDynamicIndexFromTimestampTz() {
+ IndexGenerator indexGenerator =
+ IndexGeneratorFactory.createIndexGenerator(
+ "{local_timestamp|yyyy_MM_dd_HH-ss}_index", fieldNames, dataTypes);
+ indexGenerator.open();
Review Comment:
Nit: Do you also need to close the indexGenerator? Maybe you use a try-with-resource block here.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org