Re: [PATCH] two-arg current_setting() with fallback - Mailing list pgsql-hackers

From Jeevan Chalke
Subject Re: [PATCH] two-arg current_setting() with fallback
Date
Msg-id 20150604081754.9937.57332.pgcf@coridan.postgresql.org
Whole thread Raw
In response to Re: [PATCH] two-arg current_setting() with fallback  (Sameer Thakur <samthakur74@gmail.com>)
Responses Re: [PATCH] two-arg current_setting() with fallback  (Jeevan Chalke <jeevan.chalke@enterprisedb.com>)
List pgsql-hackers
The following review has been posted through the commitfest application:
make installcheck-world:  tested, failed
Implements feature:       tested, passed
Spec compliant:           tested, passed
Documentation:            tested, passed

I have reviewed the patch.
Here are my review comments:

1. Patch does not apply due to some recent changes in pg_proc.h

2. 
-GetConfigOptionByName(const char *name, const char **varname)
+GetConfigOptionByNameMissingOK(const char *name, const char **varname, bool missing_ok)

Will it be better if we keep the name as is and change the callers to pass
false for missing_ok parameter?
It looks weired to have an extra #define just to avoid that.
I see countable callers and thus see NO issues changing those.

3. Oid used for new function is already used. Check unused_oids.sh.

4. Changes in builtins.h are accidental. Need to remove that.


However, code changes looks good and implements the desired feature.


The new status of this patch is: Waiting on Author



pgsql-hackers by date:

Previous
From: Nils Goroll
Date:
Subject: Re: xid wrap / optimize frozen tables?
Next
From: Jeevan Chalke
Date:
Subject: Re: [PATCH] two-arg current_setting() with fallback