Re: Cached plans and statement generalization

On Fri, Apr 28, 2017 at 6:01 AM, Konstantin Knizhnik <
k(dot)knizhnik(at)postgrespro(dot)ru> wrote:

>
> Any comments and suggestions for future improvement of this patch are
> welcome.
>

+ PG_TRY();
+ {
+ query = parse_analyze_varparams(parse_tree,
+ query_string,
+ &param_types,
+ &num_params);
+ }
+ PG_CATCH();
+ {
+ /*
+ * In case of analyze errors revert back to original query
processing
+ * and disable autoprepare for this query to avoid such
problems in future.
+ */
+ FlushErrorState();
+ if (snapshot_set) {
+ PopActiveSnapshot();
+ }
+ entry->disable_autoprepare = true;
+ undo_query_plan_changes(parse_tree, const_param_list);
+ MemoryContextSwitchTo(old_context);
+ return false;
+ }
+ PG_END_TRY();

This is definitely not a safe way of using TRY/CATCH.

+
+ /* Convert literal value to parameter value */
+ switch (const_param->literal->val.type)
+ {
+ /*
+ * Convert from integer literal
+ */
+ case T_Integer:
+ switch (ptype) {
+ case INT8OID:
+ params->params[paramno].value =
Int64GetDatum((int64)const_param->literal->val.val.ival);
+ break;
+ case INT4OID:
+ params->params[paramno].value =
Int32GetDatum((int32)const_param->literal->val.val.ival);
+ break;
+ case INT2OID:
+ if (const_param->literal->val.val.ival < SHRT_MIN
+ || const_param->literal->val.val.ival > SHRT_MAX)
+ {
+ ereport(ERROR,
+
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("smallint out of range")));
+ }
+ params->params[paramno].value =
Int16GetDatum((int16)const_param->literal->val.val.ival);
+ break;
+ case FLOAT4OID:
+ params->params[paramno].value =
Float4GetDatum((float)const_param->literal->val.val.ival);
+ break;
+ case FLOAT8OID:
+ params->params[paramno].value =
Float8GetDatum((double)const_param->literal->val.val.ival);
+ break;
+ case INT4RANGEOID:
+ sprintf(buf, "[%ld,%ld]",
const_param->literal->val.val.ival, const_param->literal->val.val.ival);
+ getTypeInputInfo(ptype, &typinput, &typioparam);
+ params->params[paramno].value =
OidInputFunctionCall(typinput, buf, typioparam, -1);
+ break;
+ default:
+ pg_lltoa(const_param->literal->val.val.ival, buf);
+ getTypeInputInfo(ptype, &typinput, &typioparam);
+ params->params[paramno].value =
OidInputFunctionCall(typinput, buf, typioparam, -1);
+ }
+ break;
+ case T_Null:
+ params->params[paramno].isnull = true;
+ break;
+ default:
+ /*
+ * Convert from string literal
+ */
+ getTypeInputInfo(ptype, &typinput, &typioparam);
+ params->params[paramno].value =
OidInputFunctionCall(typinput, const_param->literal->val.val.str,
typioparam, -1);
+ }

I don't see something with a bunch of hard-coded rules for particular type
OIDs having any chance of being acceptable.

This patch seems to duplicate a large amount of existing code. That would
be a good thing to avoid.

It could use a visit from the style police and a spell-checker, too.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company