Re: contribute pg_get_viewdef2 et al

Dave Page wrote:

>
>
>>-----Original Message-----
>>From: Andreas Pflug [mailto:Andreas(dot)Pflug(at)web(dot)de]
>>Sent: 07 May 2003 12:54
>>To: Dave Page; pgadmin-hackers(at)postgresql(dot)org; Tom Lane
>>Subject: contribute pg_get_viewdef2 et al
>>
>>
>>Dave Page wrote:
>>
>>I can imagine... That's why I didn't rely on the existence of the
>>function, but have a fully functional fallback solution.
>>
>>
>
>Which is good, but does make the code more complex. More importantly,
>consider what could happen if the user did a database upgrade to a
>version with different catalogues or whatever, but didn't upgrade the
>extra functions to new versions, just left the old libs there and
>reloaded their database. Or if we used a view or a plpgsql function that
>had the same problem.
>
>I'm not trying to be a pain in the *&% but I've been here before and
>don't particularly want to go through the ensuing pain again.
>
>
>
>>Well that doesn't make sense at all. Indentation and line
>>formatting can
>>be done quite well on the client side, as it is implemented now.
>>
>>
>
>I never really go a chance to look at the backend code, but I kinda
>figured it builds the SQL in a vaguely similar way to what we do (though
>obviously getting the info from the internal representation of the
>object rather than the system catalogues), and that the new functions
>could do the same just with appropriate \n's and spaces added for
>formatting.
>
>Or does pg_get_xxxdef2 just reformat pg_getxxxdef's output?
>
pg_get_xxxdef2 is a copy of pg_get_xxxdef, with some logic to detect
needed parentheses. The way to create the query from the internal
representation isn't too tricky, just recursively traverse a tree of
expression nodes. The pg_get_xxxdef versions won't try to find out if
the parentheses are really needed, they are added always. I'm using my
function isSimpleNode instead, that's all.

static bool isSimpleNode(Node *node, NodeTag parentNodeType)
{
if (!node)
return true;

switch (nodeTag(node))
{
// single words: always simple
case T_Var:
case T_Const:
case T_Param:

// function-like: name(..) or name[..]
case T_ArrayRef:
case T_FuncExpr:
case T_ArrayExpr:
case T_CoalesceExpr:
case T_NullIfExpr:
case T_Aggref:

// CASE keywords act as parentheses
case T_CaseExpr:
return true;

// appears simple since . has top precedence, unless parent is T_FieldSelect itself!
case T_FieldSelect:
return (parentNodeType == T_FieldSelect ? false : true);

// maybe simple, check args
case T_CoerceToDomain:
return isSimpleNode((Node*) ((CoerceToDomain*)node)->arg, T_CoerceToDomain);
case T_RelabelType:
return isSimpleNode((Node*) ((RelabelType*)node)->arg, T_RelabelType);

// depends on parent node type; needs further checking
case T_SubLink:
case T_NullTest:
case T_BooleanTest:
case T_OpExpr:
case T_DistinctExpr:
if (parentNodeType == T_BoolExpr)
return true;
// else check the same as for T_BoolExpr; no break here!
case T_BoolExpr:
switch (parentNodeType)
{
case T_ArrayRef:
case T_FuncExpr:
case T_ArrayExpr:
case T_CoalesceExpr:
case T_NullIfExpr:
case T_Aggref:
case T_CaseExpr:
return true;
default:
break;
}
return false;

// these are not completely implemented; so far, they're simple
case T_SubPlan:
case T_CoerceToDomainValue:
return true;

default:
break;
}
// those we don't know: in dubio complexo
return false;
}

>I agree your example is, umm, icky, but can you prove that your patch
>will not misintepret anything and produce bad output? Once again, isn't
>this a case of playing it safe?
>
How should I do this? How to PROVE software? The old problem.
You can have a look at the code, and say if there's a case when the
isSimpleNode function will falsely return true; in this case a wrong
query might be created. This function is intentionally made VERY simple,
so it shouldn't be too much work. I've sent this Tom, but he didn't
manage to have a look at it, I think.

>..casts..
>Yes, I do find them annoying sometimes.
>
>
Most of them can be omitted, but I haven't found a way to find out those
2 % which where coded explicitely :-( Both are stored as T_RelabelType.

>In that case storing the code worked because there were very few ALTER
>TABLE options in those days, and no CREATE OR REPLACES, and the code
>generally worked by completely recreating the modified object each time
>- which the user would always do through pgAdmin. Storing the original
>SQL in the catalogues however won't work because you have to let the
>user use whatever interface they like, and give them access to the ALTER
>statements. This of course means that your SQL is no longer correct as
>soon as someone renames a column or table, or performs any such
>modification of a named object.
>
Right, that's why this only works if the backend stores the query at
the moment it creates the plan. It's absolutely no option to stick users
to a single creation interface to have the query saved as side-effect;
this must be implemented integrally in the backend.

Regards,
Andreas