From: | Jeevan Chalke <jeevan(dot)chalke(at)gmail(dot)com> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] two-arg current_setting() with fallback |
Date: | 2015-06-04 08:17:54 |
Message-ID: | 20150604081754.9937.57332.pgcf@coridan.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Jeevan Chalke | 2015-06-04 08:25:52 | Re: [PATCH] two-arg current_setting() with fallback |
Previous Message | Nils Goroll | 2015-06-04 07:33:46 | Re: xid wrap / optimize frozen tables? |