Re: Add on_error and log_verbosity options to file_fdw

On 2024/09/26 21:57, torikoshia wrote:
> Updated the patches.

Thanks for updating the patches! I’ve made some changes based on your work, which are attached.
Barring any objections, I'm thinking to push these patches.

For patches 0001 and 0003, I ran pgindent and updated the commit message.

Regarding patch 0002:

- I updated the regression test to run ANALYZE on the file_fdw foreign table
since the on_error option also affects the ANALYZE command. To ensure test
stability, the test now runs ANALYZE with log_verbosity = 'silent'.

- I removed the code that updated the count of skipped rows for
the pg_stat_progress_copy view. As far as I know, file_fdw doesn’t
currently support tracking pg_stat_progress_copy.tuples_processed.
Supporting only tuples_skipped seems inconsistent, so I suggest creating
a separate patch to extend file_fdw to track both tuples_processed and
tuples_skipped in this view.

- I refactored the for-loop handling on_error = 'ignore' in fileIterateForeignScan()
by replacing it with a goto statement for improved readability.

- I modified file_fdw to log a NOTICE message about skipped rows at the end of
ANALYZE if any rows are skipped due to the on_error = 'ignore' setting.

Regarding the "file contains XXX rows" message reported by the ANALYZE VERBOSE
command on the file_fdw foreign table, what number should be reflected in XXX,
especially when some rows are skipped due to on_error = 'ignore'?
Currently, the count only includes valid rows, excluding any skipped rows.
I haven't modified this code yet. Should we instead count all rows
(valid and erroneous) and report that total?

I noticed the code for reporting the number of skipped rows due to
on_error = 'ignore' appears in three places. I’m considering creating
a common function for this reporting to eliminate redundancy but haven’t
implemented it yet.

- I’ve updated the commit message and run pgindent.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION