On 11/6/21 04:45, Pavel Stehule wrote:
Hi
st 3. 11. 2021 v 14:05 odesílatel Tomas Vondra
<tomas.von...@enterprisedb.com <mailto:tomas.von...@enterprisedb.com>>
napsal:
Hi,
I took a quick look at the latest patch version. In general the patch
looks pretty complete and clean, and for now I have only some basic
comments. The attached patch tweaks some of this, along with a couple
additional minor changes that I'll not discuss here.
1) Not sure why we need to call this "schema variables". Most objects
are placed in a schema, and we don't say "schema tables" for example.
And it's CREATE VARIABLE and not CREATE SCHEMA VARIABLE, so it's a bit
inconsistent.
Yes, there is inconsistency, but I think it is necessary. The name
"variable" is too generic. Theoretically we can use other adjectives
like session variables or global variables and the name will be valid.
But it doesn't describe the fundamentals of design. This is similar to
the package's variables from PL/SQL. These variables are global,
session's variables too. But the usual name is "package variables". So
schema variables are assigned to schemes, and I think a good name can be
"schema variables". But it is not necessary to repeat keyword schema in
the CREATE COMMAND.
My opinion is not too strong in this case, and I can accept just
"variables" or "session's variables" or "global variables", but I am not
sure if these names describe this feature well, because still they are
too generic. There are too many different implementations of session
global variables (see PL/SQL or T-SQL or DB2).
OK. "Session variable" seems better to me, but I'm not sure how well
that matches other databases. I'm not sure how much should we feel
constrained by naming in other databases, though.
The docs actually use "Global variables" in one place for some reason.
2) I find this a bit confusing:
SELECT non_existent_variable;
test=# select s;
ERROR: column "non_existent_variable" does not exist
LINE 1: select non_existent_variable;
I wonder if this means using SELECT to read variables is a bad idea, and
we should have a separate command, just like we have LET (instead of
just using UPDATE in some way).
I am sure so I want to use variables in SELECTs. One interesting case is
using variables in RLS.
How much more complicated would it be without the SELECT?
I prefer to fix this error message to "column or variable ... does not
exist"
Not sure it's a good idea to make the error message more ambiguous. Most
people won't use variables at all, and the message will be less clear
for them.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
