[ https://issues.apache.org/jira/browse/FLINK-8331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16307168#comment-16307168 ]
ASF GitHub Bot commented on FLINK-8331: --------------------------------------- Github user fhueske commented on a diff in the pull request: https://github.com/apache/flink/pull/5218#discussion_r159137345 --- Diff: flink-core/src/test/java/org/apache/flink/types/parser/ParserTestBase.java --- @@ -407,26 +407,47 @@ public void testStaticParseMethodWithInvalidValues() { @Test public void testEmptyFieldInIsolation() { try { - String [] emptyStrings = new String[] {"|"}; + FieldParser<T> parser = getParser(); + + byte[] bytes = "|".getBytes(ConfigConstants.DEFAULT_CHARSET); + int numRead = parser.parseField(bytes, 0, bytes.length, new byte[]{'|'}, parser.createValue()); + + assertEquals(FieldParser.ParseErrorState.EMPTY_COLUMN, parser.getErrorState()); + + if (this.allowsEmptyField()) { + assertTrue("Parser declared the empty string as invalid.", numRead != -1); + assertEquals("Invalid number of bytes read returned.", bytes.length, numRead); + } else { + assertTrue("Parser accepted the empty string.", numRead == -1); + } + } catch (Exception e) { + System.err.println(e.getMessage()); + e.printStackTrace(); + fail("Test erroneous: " + e.getMessage()); + } + } + + @Test + public void testTailingEmptyField() { + try { + String[] tailingEmptyFieldStrings = new String[]{"a|", "a||"}; FieldParser<T> parser = getParser(); - for (String emptyString : emptyStrings) { - byte[] bytes = emptyString.getBytes(ConfigConstants.DEFAULT_CHARSET); - int numRead = parser.parseField(bytes, 0, bytes.length, new byte[]{'|'}, parser.createValue()); + for (String tailingEmptyFieldString : tailingEmptyFieldStrings) { + byte[] bytes = tailingEmptyFieldString.getBytes(ConfigConstants.DEFAULT_CHARSET); + int numRead = parser.parseField(bytes, 1, bytes.length, new byte[]{'|'}, parser.createValue()); assertEquals(FieldParser.ParseErrorState.EMPTY_COLUMN, parser.getErrorState()); - if(this.allowsEmptyField()) { + if (this.allowsEmptyField()) { --- End diff -- Yes, you are right. I misinterpreted the `allowsEmptyField()` method. We need to keep it. > FieldParsers do not correctly set EMPT_COLUMN error state > --------------------------------------------------------- > > Key: FLINK-8331 > URL: https://issues.apache.org/jira/browse/FLINK-8331 > Project: Flink > Issue Type: Bug > Components: Core > Affects Versions: 1.5.0, 1.4.1 > Reporter: Fabian Hueske > Assignee: sunjincheng > > Some {{FieldParser}} do not correctly set the EMPTY_COLUMN error state if a > field is empty. > Instead, they try to parse the field value from an empty String which fails, > e.g., in case of the {{DoubleParser}} with a {{NumberFormatException}}. > The {{RowCsvInputFormat}} has a flag to interpret empty fields as {{null}} > values. The implementation requires that all {{FieldParser}} correctly return > the EMPTY_COLUMN error state in case of an empty field. > Affected {{FieldParser}}: > - BigDecParser > - BigIntParser > - DoubleParser > - FloatParser > - SqlDateParser > - SqlTimeParser > - SqlTimestampParser -- This message was sent by Atlassian JIRA (v6.4.14#64029)