On Tue, 2021-06-01 at 15:38 +0300, Aleksander Alekseev wrote:
> I came across this patch and noticed that it rotted a little,
> especially after removing inheritance_planner() in 86dc9005. I
> managed to resolve the conflicts on current `master` (eb89cb43), see
> the attached patch. The code compiles but doesn't pass the tests. I'm
> currently in the process of reviewing it and didn't figure out what
> the issue is yet. Just wanted to let you know.
Hi Alexsander, thanks!
In your patch's transformInsertStmt(), I see what I think is an
extraneous call to transformReturningList() right before the ON
CONFLICT processing. That call is already done later in the function,
during the RETURNING processing (this change came in with 6c0373ab77).
Other than that, your rebased patch looks the same as mine.
> I also believe changing the patch status to "Waiting on Author"
> would be appropriate.
Agreed. I'm going to double-check with Deep that the new calls
to table_tuple_fetch_row_version() should be projecting the full row,
then post an updated patch some time next week.
Thanks again!
--Jacob
Hi,
+ return relation->rd_tableam->scan_begin_with_column_projection(relation, snapshot, 0, NULL,
+ parallel_scan, flags, proj);
scan_begin_with_column_projection() adds a parameter to scan_begin().
Can scan_begin() be enhanced with this projection parameter ?
Otherwise in the future we may have scan_begin_with_column_projection_with_x_y ...
+ /* Make sure the the new slot is not dependent on the original tuple */
Double 'the' in the comment. More than one place with duplicate 'the' in the patch.
+typedef struct neededColumnContext
+{
+ Bitmapset **mask;
+ int n;
Should field n be named ncol ? 'n' seems too general.
+ * TODO: Remove this hack!! This should be done once at the start of the tid scan.
Would the above be addressed in the next patch ?
Toward the end of extract_scan_columns():
+ bms_free(rte->scanCols);
+ rte->scanCols = bms_make_singleton(0);
+ break;
Should 'goto outer;' be in place of 'break;' (since rte->scanCols has been assigned for whole-row) ?
Cheers
