Fix the CSV serialization/deserialization bug for Time

[yariks5s]

Closes #82407

Changelog category (leave one):

• Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix a bug when incorrect type order during CSV deserialization led to the LOGICAL_ERROR

Documentation entry for user-facing changes

• Documentation is written (mandatory for new features)

[clickhouse-gh]

Workflow [PR], commit [2ffa20f]

Summary: ❌

job_name	test_name	status	info	comment
Integration tests (arm_binary, distributed plan, 2/4)		failure
	test_trace_log_memory_context/test.py::test_memory_context_in_trace_log	FAIL
Integration tests (arm_binary, distributed plan, 3/4)		failure
	test_storage_rabbitmq/test.py::test_rabbitmq_tsv_with_delimiter	FAIL
	test_storage_rabbitmq/test.py::test_rabbitmq_macros	FAIL
	test_keeper_memory_soft_limit/test.py::test_soft_limit_create	FAIL

[alexey-milovidov]

This looks incorrect, as the buffer could have changed.

[alexey-milovidov]

Why do we need this change?

[alexey-milovidov]

@yariks5s, good news, the error was not anywhere around your code for the Time data type, and these changes are unneeded.

The error was introduced in this PR: #51716, @Avogar
It was impossible to implement that feature correctly, and it does not make sense in general.

I recommend @Avogar deprecating that old mis-feature.

[yariks5s]

I think we can close this PR now as it will not fix the problem completely

[Avogar]

The problem not in the #51716, the problem is here:

ClickHouse/src/DataTypes/Serializations/SerializationTime.cpp

Line 202 in d8595d7

throwUnexpectedDataAfterParsedValue(column, istr, settings, "Time");

Function throwUnexpectedDataAfterParsedValue calls column->popBack(1), but here in SerializationTime::deserializeTextCSV we don't add value to the column at the time we call this function. So we delete previous row and end up with 1 row less in the chunk.

Also I noticed that input_format_allow_errors_num doesn't work in this case because UNEXPECTED_DATA_AFTER_PARSED_VALUE is not listed as parsing error here:

ClickHouse/src/Processors/Formats/IRowInputFormat.cpp

Lines 36 to 56 in d8595d7

	bool isParseError(int code)
	{
	return code == ErrorCodes::CANNOT_PARSE_INPUT_ASSERTION_FAILED
	|| code == ErrorCodes::CANNOT_PARSE_QUOTED_STRING
	|| code == ErrorCodes::CANNOT_PARSE_DATE
	|| code == ErrorCodes::CANNOT_PARSE_DATETIME
	|| code == ErrorCodes::CANNOT_PARSE_NUMBER
	|| code == ErrorCodes::CANNOT_PARSE_UUID
	|| code == ErrorCodes::CANNOT_PARSE_BOOL
	|| code == ErrorCodes::CANNOT_READ_ARRAY_FROM_TEXT
	|| code == ErrorCodes::CANNOT_READ_MAP_FROM_TEXT
	|| code == ErrorCodes::CANNOT_READ_ALL_DATA
	|| code == ErrorCodes::TOO_LARGE_STRING_SIZE
	|| code == ErrorCodes::ARGUMENT_OUT_OF_BOUND /// For Decimals
	|| code == ErrorCodes::INCORRECT_DATA /// For some ReadHelpers
	|| code == ErrorCodes::CANNOT_PARSE_DOMAIN_VALUE_FROM_STRING
	|| code == ErrorCodes::CANNOT_PARSE_IPV4
	|| code == ErrorCodes::CANNOT_PARSE_IPV6
	|| code == ErrorCodes::UNKNOWN_ELEMENT_OF_ENUM
	|| code == ErrorCodes::CANNOT_PARSE_ESCAPE_SEQUENCE;
	}

The fix now is correct.

[yariks5s]

PR / Integration tests (arm_binary, distributed plan, 2/4) (pull_request) - #87995

Integration tests (arm_binary, distributed plan, 3/4) - #71049, #87877