deniskuzZ commented on PR #6794: URL: https://github.com/apache/hive/pull/6794#issuecomment-6214743884
Thanks, all the main points are addressed. A few cleanups before merge: - Please update the PR description: it still says INSERT rows are excluded from the MERGE count and describes the all-writers condition, which no longer matches the code. - MultiInsertSqlGenerator.getRowIdIndex() / getRowIdType() are unused, please drop them. - Leftover whitespace-only hunks in CopyOnWriteMergeRewriter and the reflowed license header in COWWithClauseBuilder. - As discussed earlier, all writers of one sink are created by the same output format and WriterBuilder picks the CoW writer per table/operation, so a sink can’t mix writer types. Could we drop `hasNonAffectedRowsWriter` and keep just `affectedRowCount` (with -1 meaning “not reported”)? `outWritersClosed` is fine to keep. nit: row_count.set(...) moved inside if (!abort), so RECORDS_OUT is no longer set on abort; it could stay where it was, only modifiedRowCount needs to be set after the writers are closed. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
