>
> Thanks. Committed after applying some tweaks to it. I have noticed
> that you forgot numeric_int4_opt_error() in the set.
Oops. Thanks for the commit, Michael.
Regards,
Jeevan Ladhe
On Mon, Aug 05, 2019 at 09:15:02AM +0530, Jeevan Ladhe wrote:
> Please find attached patch with the changes to RETURN_ERROR and
> it's references in float.c
Thanks. Committed after applying some tweaks to it. I have noticed
that you forgot numeric_int4_opt_error() in the set.
--
Michael
On Sun, Aug 4, 2019 at 12:13 PM Michael Paquier wrote:
> On Sat, Aug 03, 2019 at 11:57:01PM -0400, Alvaro Herrera wrote:
> > Can we please change the macro definition so that have_error is one of
> > the arguments? Having the variable be used inside the macro definition
> > but not appear
On Sat, Aug 03, 2019 at 11:57:01PM -0400, Alvaro Herrera wrote:
> Can we please change the macro definition so that have_error is one of
> the arguments? Having the variable be used inside the macro definition
> but not appear literally in the call is quite confusing.
Good idea. This needs some
On 2019-Aug-04, Jeevan Ladhe wrote:
> Sure Michael, in the attached patch I have reverted the checks from
> pg_lsn_in_internal() and added Assert() per my original patch.
Can we please change the macro definition so that have_error is one of
the arguments? Having the variable be used inside the
Sure Michael, in the attached patch I have reverted the checks from
pg_lsn_in_internal() and added Assert() per my original patch.
Regards,
Jeevan Ladhe
0001-Make-have_error-initialization-more-defensive-v2.patch
Description: Binary data
On Thu, Aug 01, 2019 at 02:10:08PM +0530, Jeevan Ladhe wrote:
> The only thing is that, if the caller cares about the error during
> the parsing or not.
That's where the root of the problem is. We should really make things
so as the caller of this routine cares about errors. With your patch
a
Hi Michael,
On Thu, Aug 1, 2019 at 1:51 PM Michael Paquier wrote:
> On Thu, Aug 01, 2019 at 12:39:26PM +0530, Jeevan Ladhe wrote:
> > Here is a patch that takes care of addressing the flag issue including
> > pg_lsn_in_internal() and others.
>
> Your original patch for pg_lsn_in_internal() was
On Thu, Aug 01, 2019 at 12:39:26PM +0530, Jeevan Ladhe wrote:
> Here is a patch that takes care of addressing the flag issue including
> pg_lsn_in_internal() and others.
Your original patch for pg_lsn_in_internal() was right IMO, and the
new one is not. In the numeric and float code paths, we
Hi Michael,
> I will further check if by mistake any further commits have removed
> > references to assignments from float8in_internal_opt_error(),
> > evaluate it, and set out a patch.
>
> Thanks, Jeevan!
>
Here is a patch that takes care of addressing the flag issue including
On Thu, Aug 01, 2019 at 11:14:32AM +0530, Jeevan Ladhe wrote:
> Sure, agree, it makes sense to address float8in_internal_opt_error(),
> there might be more occurrences of such instances in other functions
> as well. I think if we agree, as and when encounter them while touching
> those areas we
Hi Michael,
What is more dangerous with float8in_internal_opt_error() is, it has
> the have_error flag, which is never ever set or used in that function.
> Further
> more risks are - the callers of this function e.g.
> executeItemOptUnwrapTarget()
> are passing a non-null pointer to it(default
Hi Michael,
> On further review of the first patch, I think that it could be a good
> idea to apply the same safeguards within float8in_internal_opt_error.
> Jeevan, what do you think?
>
Sure, agree, it makes sense to address float8in_internal_opt_error(),
there might be more occurrences of
On the topic of pg_lsn, I recently noticed that there's no
operator(+)(pg_lsn,bigint) nor is there an operator(-)(pg_lsn,bigint) so
you can't compute offsets easily. We don't have a cast between pg_lsn and
bigint because we don't expose an unsigned bigint type in SQL, so you can't
work around it
On Wed, Jul 31, 2019 at 09:51:30AM +0900, Michael Paquier wrote:
> I am adding Peter Eisentraut in CC as 21f428e is his commit. I think
> that the first patch is a good idea, so I would be fine to apply it,
> but let's see the original committer's opinion first.
On further review of the first
On Tue, Jul 30, 2019 at 6:06 PM Robert Haas wrote:
> On Tue, Jul 30, 2019 at 4:52 AM Jeevan Ladhe
> wrote:
> > My only concern was something that we internally treat as invalid, why do
> > we allow, that as a valid value for that type. While I am not trying to
> > reinvent the wheel here, I am
On Tue, Jul 30, 2019 at 02:22:30PM +0530, Jeevan Ladhe wrote:
> On Tue, Jul 30, 2019 at 9:42 AM Michael Paquier wrote:
>> Agreed about making the code more defensive as you do. I would keep
>> the initialization in check_recovery_target_lsn and pg_lsn_in_internal
>> though. That does not hurt
On Tue, Jul 30, 2019 at 4:52 AM Jeevan Ladhe
wrote:
> My only concern was something that we internally treat as invalid, why do
> we allow, that as a valid value for that type. While I am not trying to
> reinvent the wheel here, I am trying to understand if there had been any
> idea behind this
Hi Michael,
Thanks for your inputs, really appreciate.
On Tue, Jul 30, 2019 at 9:42 AM Michael Paquier wrote:
> On Mon, Jul 29, 2019 at 10:55:29PM +0530, Jeevan Ladhe wrote:
> > I am attaching a patch that makes sure that *have_error is set to false
> in
> > pg_lsn_in_internal() itself, rather
On Mon, Jul 29, 2019 at 10:55:29PM +0530, Jeevan Ladhe wrote:
> I am attaching a patch that makes sure that *have_error is set to false in
> pg_lsn_in_internal() itself, rather than being caller dependent.
Agreed about making the code more defensive as you do. I would keep
the initialization in
20 matches
Mail list logo
