Re: Report error position in partition bound check - Mailing list pgsql-hackers

From Ashutosh Bapat
Subject Re: Report error position in partition bound check
Date
Msg-id CAG-ACPVSOpL3uP2R3rK-ZXC0aq3D=_NaeNay6m-OcUrkyZeKhA@mail.gmail.com
Whole thread Raw
In response to Re: Report error position in partition bound check  (Amit Langote <amitlangote09@gmail.com>)
Responses Re: Report error position in partition bound check  (Amit Langote <amitlangote09@gmail.com>)
List pgsql-hackers


On Thu, 17 Sep 2020 at 13:06, Amit Langote <amitlangote09@gmail.com> wrote:
Hi Ashutosh,

I had forgotten about this thread, but Michael's ping email brought it
to my attention.

Thanks Amit for addressing comments.

@@ -4256,5 +4256,8 @@ transformPartitionBoundValue(ParseState *pstate, Node *val,
  if (!IsA(value, Const))
  elog(ERROR, "could not evaluate partition bound expression");
 
+ /* Preserve parser location information. */
+ ((Const *) value)->location = exprLocation(val);
+
  return (Const *) value;
 }

This caught my attention and I was wondering whether transformExpr() itself should transfer the location from input expression to the output expression. Some minions of transformExprRecurse() seem to be doing that. The change here may be an indication that some of them are not doing this. In that case may be it's better to find those and fix rather than a white-wash fix here. In what case did we find that location was not set by transformExpr? Sorry for not catching this earlier.

/* New lower bound is certainly >= bound at offet. */
offet/offset? But this comment is implied by the comment just two lines above. So I am not sure it's really needed.

/* Fetch the problem bound from lower datums list. */
This is fetching problematic key value rather than the whole problematic bound. I think the comment would be useful if it explains why cmpval -1 th key is problematic but then that's evident from the prologue of partition_rbound_cmp() so I am not sure if this comment is really required. For example, we aren't adding a comment here
+ overlap_location = ((PartitionRangeDatum *)
+ list_nth(spec->upperdatums, -cmpval - 1))->location;

--
Best Wishes,
Ashutosh

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: pg_logging_init() can return ENOTTY with TAP tests
Next
From: Amit Khandekar
Date:
Subject: Re: Redundant tuple copy in tqueueReceiveSlot()