Bugfix volalite functions#47

[onderkalaci]

This pull request prevents WHERE clause statements to have VOLATILE or MUTABLE functions.

Fixes #47

Review tasks

• [x] Fix comment in test
• [x] Rework area that used ternary expression
• [x] Remove call to contain_volatile_functions
• [x] Add test with mutable but not volatile function such as current_timestamp

[onderkalaci]

@jasonmp85 I think we can do a similar change for this line. In other words, instead of checking

!IsA(targetEntry->expr, Const)

We can do something like:

contain_mutable_functions((targetEntry->exp) ||
    contain_volatile_functions((targetEntry->exp))  

But, since the issue only considers QUALs, I didn't want to do that here.

[jasonmp85]

@onderkalaci this looks good and after the small style changes (and removing the redundant call), it's a :shipit:.

[jasonmp85]

Accidentally closed, sorry!

[jasonmp85]

@onderkalaci on the topic of !IsA(targetEntry->expr, Const), we already know any function calls we see in the target list must be mutable. This is because we have already called the standard_planner on the query. That in turn calls eval_const_expressions, which eagerly replaces any immutable functions with their output (this also applies to the quals; for instance, doing an UPDATE … WHERE foo = (1 + 1) becomes UPDATE … WHERE foo = 2).

So for target lists, we only accept constants, but for quals, there might be a column reference (LHS), operator, and value (RHS). In that case, the standard planner may have collapsed the RHS to a constant (if, as above it's something like 1+1), but the operator call is still there. But e.g. = is IMMUTABLE so it needs to make it through.

[jasonmp85]

So I guess I'm saying the walker is overkill for the target list but is needed for the quals, so what we have here is now complete.

[jasonmp85]

Added checklist up to. Fix and submit the changes and I'll merge tomorrow if all looks good.

[onderkalaci]

@jasonmp85 all your comments make sense and I applied the changes.

[jasonmp85]

Looks good. Merging.