Re: Add Boolean node
On 13.01.22 10:48, Pavel Stehule wrote: There are not objection from me or from community I'll mark this patch as ready for committer This patch set has been committed.
Re: Add Boolean node
Hi po 3. 1. 2022 v 14:18 odesílatel Peter Eisentraut < peter.eisentr...@enterprisedb.com> napsal: > > On 03.01.22 12:04, Peter Eisentraut wrote: > > On 27.12.21 10:02, Peter Eisentraut wrote: > >> This patch adds a new node type Boolean, to go alongside the "value" > >> nodes Integer, Float, String, etc. This seems appropriate given that > >> Boolean values are a fundamental part of the system and are used a lot. > >> > >> Before, SQL-level Boolean constants were represented by a string with > >> a cast, and internal Boolean values in DDL commands were usually > >> represented by Integer nodes. This takes the place of both of these > >> uses, making the intent clearer and having some amount of type safety. > > > > Here is an update of this patch set based on the feedback. First, I > > added a patch that makes some changes in AlterRole() that my original > > patch might have broken or at least made more confusing. Unlike in > > CreateRole(), we use three-valued logic here, so that a variable like > > issuper would have 0 = no, 1 = yes, -1 = not specified, keep previous > > value. I'm simplifying this, by instead using the dissuper etc. > > variables to track whether a setting was specified. This makes > > everything a bit simpler and makes the subsequent patch easier. > > > > Second, I added the suggest by Tom Lane to rename to fields in the > > used-to-be-Value nodes to be different in each node type (ival, fval, > > etc.). I agree that this makes things a bit cleaner and reduces the > > changes of mixups. > > > > And third, the original patch that introduces the Boolean node with some > > small changes based on the feedback. > > Another very small update, attempting to appease the cfbot. This is almost trivial patch There are not problems with patching, compilation and tests make check-world passed There are not objection from me or from community I'll mark this patch as ready for committer Regards Pavel
Re: Add Boolean node
95c3631c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -8332,7 +8332,7 @@ flatten_set_variable_args(const char *name, List *args)
break;
case T_Float:
/* represented as a string, so just copy it */
- appendStringInfoString(&buf, castNode(Float,
&con->val)->val);
+ appendStringInfoString(&buf, castNode(Float,
&con->val)->fval);
break;
case T_String:
val = strVal(&con->val);
diff --git a/src/include/nodes/value.h b/src/include/nodes/value.h
index 8b71b510eb..cc3a4a639c 100644
--- a/src/include/nodes/value.h
+++ b/src/include/nodes/value.h
@@ -28,7 +28,7 @@
typedef struct Integer
{
NodeTag type;
- int val;
+ int ival;
} Integer;
/*
@@ -45,24 +45,24 @@ typedef struct Integer
typedef struct Float
{
NodeTag type;
- char *val;
+ char *fval;
} Float;
typedef struct String
{
NodeTag type;
- char *val;
+ char *sval;
} String;
typedef struct BitString
{
NodeTag type;
- char *val;
+ char *bsval;
} BitString;
-#define intVal(v) (castNode(Integer, v)->val)
-#define floatVal(v)atof(castNode(Float, v)->val)
-#define strVal(v) (castNode(String, v)->val)
+#define intVal(v) (castNode(Integer, v)->ival)
+#define floatVal(v)atof(castNode(Float, v)->fval)
+#define strVal(v) (castNode(String, v)->sval)
extern Integer *makeInteger(int i);
extern Float *makeFloat(char *numericStr);
--
2.34.1
From e4278f65fe7c9d9c88af3abd26604d27f124c752 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 3 Jan 2022 14:16:04 +0100
Subject: [PATCH v3 3/3] Add Boolean node
Before, SQL-level boolean constants were represented by a string with
a cast, and internal Boolean values in DDL commands were usually
represented by Integer nodes. This takes the place of both of these
uses, making the intent clearer and having some amount of type safety.
---
contrib/postgres_fdw/postgres_fdw.c | 32 +++---
src/backend/commands/define.c | 2 +
src/backend/commands/functioncmds.c | 14 +--
src/backend/commands/sequence.c | 4 +-
src/backend/commands/tsearchcmds.c| 9 ++
src/backend/commands/user.c | 28 +++---
src/backend/nodes/copyfuncs.c | 16 +++
src/backend/nodes/equalfuncs.c| 11 +++
src/backend/nodes/nodeFuncs.c | 1 +
src/backend/nodes/outfuncs.c | 8 ++
src/backend/nodes/read.c | 9 +-
src/backend/nodes/value.c | 12 +++
src/backend/parser/gram.y | 99 ++-
src/backend/parser/parse_node.c | 8 ++
src/backend/replication/repl_gram.y | 14 +--
src/include/nodes/nodes.h | 1 +
src/include/nodes/parsenodes.h| 1 +
src/include/nodes/value.h | 8 ++
src/test/isolation/expected/ri-trigger.out| 60 +--
.../regress/expected/create_function_3.out| 2 +-
20 files changed, 211 insertions(+), 128 deletions(-)
diff --git a/contrib/postgres_fdw/postgres_fdw.c
b/contrib/postgres_fdw/postgres_fdw.c
index fa9a099f13..4d61d88d7b 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -103,7 +103,7 @@ enum FdwModifyPrivateIndex
FdwModifyPrivateTargetAttnums,
/* Length till the end of VALUES clause (as an Integer node) */
FdwModifyPrivateLen,
- /* has-returning flag (as an Integer node) */
+ /* has-returning flag (as a Boolean node) */
FdwModifyPrivateHasReturning,
/* Integer list of attribute numbers retrieved by RETURNING */
FdwModifyPrivateRetrievedAttrs
@@ -122,11 +122,11 @@ enum FdwDirectModifyPrivateIndex
{
/* SQL statement to execute remotely (as a String node) */
FdwDirectModifyPrivateUpdateSql,
- /* has-returning flag (as an Integer node) */
+ /* has-returning flag (as a Boolean node) */
FdwDirectModifyPrivateHasReturning,
/* Integer list of attribute numbers retrieved by RETURNING */
FdwDirectModifyPrivateRetrievedAttrs,
- /* set-processed flag (as an Integer node) */
+ /* set-processed flag (as a Boolean node) */
FdwDirectModifyPrivateSetProcessed
};
@@ -280,9 +280,9 @@ typedef struct PgFdwAnalyzeState
*/
enum FdwPathPrivateIndex
{
- /* has-final-sort flag (as an Integer node) */
+ /* has-final-sort flag (as a Boolean
Re: Add Boolean node
latten_set_variable_args(const char *name, List *args)
break;
case T_Float:
/* represented as a string, so just copy it */
- appendStringInfoString(&buf, castNode(Float,
&con->val)->val);
+ appendStringInfoString(&buf, castNode(Float,
&con->val)->fval);
break;
case T_String:
val = strVal(&con->val);
diff --git a/src/include/nodes/value.h b/src/include/nodes/value.h
index 8b71b510eb..cc3a4a639c 100644
--- a/src/include/nodes/value.h
+++ b/src/include/nodes/value.h
@@ -28,7 +28,7 @@
typedef struct Integer
{
NodeTag type;
- int val;
+ int ival;
} Integer;
/*
@@ -45,24 +45,24 @@ typedef struct Integer
typedef struct Float
{
NodeTag type;
- char *val;
+ char *fval;
} Float;
typedef struct String
{
NodeTag type;
- char *val;
+ char *sval;
} String;
typedef struct BitString
{
NodeTag type;
- char *val;
+ char *bsval;
} BitString;
-#define intVal(v) (castNode(Integer, v)->val)
-#define floatVal(v)atof(castNode(Float, v)->val)
-#define strVal(v) (castNode(String, v)->val)
+#define intVal(v) (castNode(Integer, v)->ival)
+#define floatVal(v)atof(castNode(Float, v)->fval)
+#define strVal(v) (castNode(String, v)->sval)
extern Integer *makeInteger(int i);
extern Float *makeFloat(char *numericStr);
--
2.34.1
From eb4bdfc511f1c0f2e8f3971699c33fcfb8cf6105 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 3 Jan 2022 11:19:52 +0100
Subject: [PATCH v2 3/3] Add Boolean node
Before, SQL-level boolean constants were represented by a string with
a cast, and internal Boolean values in DDL commands were usually
represented by Integer nodes. This takes the place of both of these
uses, making the intent clearer and having some amount of type safety.
---
contrib/postgres_fdw/postgres_fdw.c | 32 +++---
src/backend/commands/define.c | 2 +
src/backend/commands/functioncmds.c | 14 +--
src/backend/commands/sequence.c | 4 +-
src/backend/commands/tsearchcmds.c| 9 ++
src/backend/commands/user.c | 28 +++---
src/backend/nodes/copyfuncs.c | 16 +++
src/backend/nodes/equalfuncs.c| 11 +++
src/backend/nodes/nodeFuncs.c | 1 +
src/backend/nodes/outfuncs.c | 8 ++
src/backend/nodes/read.c | 9 +-
src/backend/nodes/value.c | 12 +++
src/backend/parser/gram.y | 98 +--
src/backend/parser/parse_node.c | 8 ++
src/backend/replication/repl_gram.y | 14 +--
src/include/nodes/nodes.h | 1 +
src/include/nodes/parsenodes.h| 1 +
src/include/nodes/value.h | 8 ++
src/test/isolation/expected/ri-trigger.out| 60 ++--
.../regress/expected/create_function_3.out| 2 +-
20 files changed, 210 insertions(+), 128 deletions(-)
diff --git a/contrib/postgres_fdw/postgres_fdw.c
b/contrib/postgres_fdw/postgres_fdw.c
index fa9a099f13..4d61d88d7b 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -103,7 +103,7 @@ enum FdwModifyPrivateIndex
FdwModifyPrivateTargetAttnums,
/* Length till the end of VALUES clause (as an Integer node) */
FdwModifyPrivateLen,
- /* has-returning flag (as an Integer node) */
+ /* has-returning flag (as a Boolean node) */
FdwModifyPrivateHasReturning,
/* Integer list of attribute numbers retrieved by RETURNING */
FdwModifyPrivateRetrievedAttrs
@@ -122,11 +122,11 @@ enum FdwDirectModifyPrivateIndex
{
/* SQL statement to execute remotely (as a String node) */
FdwDirectModifyPrivateUpdateSql,
- /* has-returning flag (as an Integer node) */
+ /* has-returning flag (as a Boolean node) */
FdwDirectModifyPrivateHasReturning,
/* Integer list of attribute numbers retrieved by RETURNING */
FdwDirectModifyPrivateRetrievedAttrs,
- /* set-processed flag (as an Integer node) */
+ /* set-processed flag (as a Boolean node) */
FdwDirectModifyPrivateSetProcessed
};
@@ -280,9 +280,9 @@ typedef struct PgFdwAnalyzeState
*/
enum FdwPathPrivateIndex
{
- /* has-final-sort flag (as an Integer node) */
+ /* has-final-sort flag (as a Boolean node) */
FdwPathPrivateHasFinalSort,
- /* has-limit flag (as an Integer node) */
+ /* has-limit flag (as a
Re: Add Boolean node
On 29.12.21 21:32, Andres Freund wrote: On 2021-12-27 09:53:32 -0500, Tom Lane wrote: Didn't really read the patch in any detail, but I did have one idea: I think that the different things-that-used-to-be-Value-nodes ought to use different field names, say ival, rval, bval, sval not just "val". That makes it more likely that you'd catch any code that is doing the wrong thing and not going through one of the access macros. If we go around changing all these places, it might be worth to also change Integer to be a int64 instead of an int. I was actually looking into that, when I realized that most uses of Integer were actually Booleans. Hence the current patch to clear those fake Integers out of the way. I haven't gotten to analyze the int64 question any further, but it should be easier hereafter.
Re: Add Boolean node
Hi,
On 2021-12-27 10:02:14 +0100, Peter Eisentraut wrote:
> This patch adds a new node type Boolean, to go alongside the "value" nodes
> Integer, Float, String, etc. This seems appropriate given that Boolean
> values are a fundamental part of the system and are used a lot.
>
> Before, SQL-level Boolean constants were represented by a string with
> a cast, and internal Boolean values in DDL commands were usually represented
> by Integer nodes. This takes the place of both of these uses, making the
> intent clearer and having some amount of type safety.
This annoyed me plenty of times before, plus many.
> From 4e1ef56b5443fa11d981eb6e407dfc7c244dc60e Mon Sep 17 00:00:00 2001
> From: Peter Eisentraut
> Date: Mon, 27 Dec 2021 09:52:05 +0100
> Subject: [PATCH v1] Add Boolean node
>
> Before, SQL-level boolean constants were represented by a string with
> a cast, and internal Boolean values in DDL commands were usually
> represented by Integer nodes. This takes the place of both of these
> uses, making the intent clearer and having some amount of type safety.
> ---
> ...
> 20 files changed, 210 insertions(+), 126 deletions(-)
This might be easier to review if there were one patch adding the Boolean
type, and then a separate one converting users?
> diff --git a/src/backend/commands/tsearchcmds.c
> b/src/backend/commands/tsearchcmds.c
> index c47a05d10d..b7261a88d4 100644
> --- a/src/backend/commands/tsearchcmds.c
> +++ b/src/backend/commands/tsearchcmds.c
> @@ -1742,6 +1742,15 @@ buildDefItem(const char *name, const char *val, bool
> was_quoted)
> return makeDefElem(pstrdup(name),
> (Node *)
> makeFloat(pstrdup(val)),
> -1);
> +
> + if (strcmp(val, "true") == 0)
> + return makeDefElem(pstrdup(name),
> +(Node *)
> makeBoolean(true),
> +-1);
> + if (strcmp(val, "false") == 0)
> + return makeDefElem(pstrdup(name),
> +(Node *)
> makeBoolean(false),
> +-1);
> }
> /* Just make it a string */
> return makeDefElem(pstrdup(name),
Hm. defGetBoolean() interprets "true", "false", "on", "off" as booleans. ISTM
we shouldn't invent different behaviours for individual subsystems?
> --- a/src/backend/nodes/outfuncs.c
> +++ b/src/backend/nodes/outfuncs.c
> @@ -3433,6 +3433,12 @@ _outFloat(StringInfo str, const Float *node)
> appendStringInfoString(str, node->val);
> }
>
> +static void
> +_outBoolean(StringInfo str, const Boolean *node)
> +{
> + appendStringInfoString(str, node->val ? "true" : "false");
> +}
Any reason not to use 't' and 'f' instead? It seems unnecessary to bloat the
node output by the longer strings, and it makes parsing more expensive
too:
> --- a/src/backend/nodes/read.c
> +++ b/src/backend/nodes/read.c
> @@ -283,6 +283,8 @@ nodeTokenType(const char *token, int length)
> retval = RIGHT_PAREN;
> else if (*token == '{')
> retval = LEFT_BRACE;
> + else if (strcmp(token, "true") == 0 || strcmp(token, "false") == 0)
> + retval = T_Boolean;
> else if (*token == '"' && length > 1 && token[length - 1] == '"')
> retval = T_String;
> else if (*token == 'b')
Before this could be implemented as a jump table, not now it can't easily be
anymore.
> diff --git a/src/test/isolation/expected/ri-trigger.out
> b/src/test/isolation/expected/ri-trigger.out
> index 842df80a90..db85618bef 100644
> --- a/src/test/isolation/expected/ri-trigger.out
> +++ b/src/test/isolation/expected/ri-trigger.out
> @@ -4,9 +4,9 @@ starting permutation: wxry1 c1 r2 wyrx2 c2
> step wxry1: INSERT INTO child (parent_id) VALUES (0);
> step c1: COMMIT;
> step r2: SELECT TRUE;
> -bool
> -
> -t
> +?column?
> +
> +t
> (1 row)
This doesn't seem great. It might be more consistent ("SELECT 1" doesn't end
up with 'integer' as column name), but this still seems like an unnecessarily
large user-visible change for an internal data-representation change?
Greetings,
Andres Freund
Re: Add Boolean node
Andres Freund writes: > If we go around changing all these places, it might be worth to also change > Integer to be a int64 instead of an int. Meh ... that would have some non-obvious consequences, I think, at least if you tried to make the grammar make use of the extra width (it'd change the type resolution behavior for integer-ish literals). I think it's better to keep it as plain int. regards, tom lane
Re: Add Boolean node
On 2021-12-27 09:53:32 -0500, Tom Lane wrote: > Didn't really read the patch in any detail, but I did have one idea: > I think that the different things-that-used-to-be-Value-nodes ought to > use different field names, say ival, rval, bval, sval not just "val". > That makes it more likely that you'd catch any code that is doing the > wrong thing and not going through one of the access macros. If we go around changing all these places, it might be worth to also change Integer to be a int64 instead of an int.
Re: Add Boolean node
=?UTF-8?B?Sm9zZWYgxaBpbcOhbmVr?= writes: > po 27. 12. 2021 v 16:10 odesílatel Alvaro Herrera > napsal: >> Hmm, interesting side-effect: we no longer assign a column name in this >> case so it remains "?column?", just like it happens for other datatypes. >> This seems okay to me. (This is also what causes the changes in the >> isolationtester expected output.) > This seems to be caused by a change of makeBoolAConst function. I was > thinking for a while about the potential backward compatibility > problems, but I wasn't able to find any. In theory this could break some application that's expecting "SELECT ..., true, ..." to return a column name of "bool" rather than "?column?". The risk of that being a problem in practice seems rather low, though. It certainly seems like a wart that you get a type name for that but not for other sorts of literals such as 1 or 2.4, so I'm okay with the change. regards, tom lane
Re: Add Boolean node
po 27. 12. 2021 v 16:10 odesílatel Alvaro Herrera
napsal:
>
> On 2021-Dec-27, Peter Eisentraut wrote:
>
> > This patch adds a new node type Boolean, to go alongside the "value" nodes
> > Integer, Float, String, etc. This seems appropriate given that Boolean
> > values are a fundamental part of the system and are used a lot.
>
> I like the idea. I'm surprised that there is no notational savings in
> the patch, however.
>
> > diff --git a/src/test/regress/expected/create_function_3.out
> > b/src/test/regress/expected/create_function_3.out
> > index 3a4fd45147..e0c4bee893 100644
> > --- a/src/test/regress/expected/create_function_3.out
> > +++ b/src/test/regress/expected/create_function_3.out
> > @@ -403,7 +403,7 @@ SELECT pg_get_functiondef('functest_S_13'::regproc);
> >LANGUAGE sql+
> > BEGIN ATOMIC +
> >SELECT 1; +
> > - SELECT false AS bool; +
> > + SELECT false; +
> > END +
>
> Hmm, interesting side-effect: we no longer assign a column name in this
> case so it remains "?column?", just like it happens for other datatypes.
> This seems okay to me. (This is also what causes the changes in the
> isolationtester expected output.)
This seems to be caused by a change of makeBoolAConst function. I was
thinking for a while about the potential backward compatibility
problems, but I wasn't able to find any.
> --
> Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/
> "Ni aún el genio muy grande llegaría muy lejos
> si tuviera que sacarlo todo de su propio interior" (Goethe)
>
>
Re: Add Boolean node
On 27.12.21 14:15, Ashutosh Bapat wrote: That looks like a good change. I wonder what motivates that now? Why wasn't it added when the usages grew? Are there more Boolean usages planned? Mainly, I was looking at Integer/makeInteger() and noticed that most uses of those weren't actually integers but booleans. This change makes it clearer which is which.
Re: Add Boolean node
Hi, For buildDefItem(): + if (strcmp(val, "true") == 0) + return makeDefElem(pstrdup(name), + (Node *) makeBoolean(true), + -1); + if (strcmp(val, "false") == 0) Should 'TRUE' / 'FALSE' be considered above ? - issuper = intVal(dissuper->arg) != 0; + issuper = boolVal(dissuper->arg) != 0; Can the above be written as (since issuper is a bool): + issuper = boolVal(dissuper->arg); Cheers
Re: Add Boolean node
On 2021-Dec-27, Peter Eisentraut wrote:
> This patch adds a new node type Boolean, to go alongside the "value" nodes
> Integer, Float, String, etc. This seems appropriate given that Boolean
> values are a fundamental part of the system and are used a lot.
I like the idea. I'm surprised that there is no notational savings in
the patch, however.
> diff --git a/src/test/regress/expected/create_function_3.out
> b/src/test/regress/expected/create_function_3.out
> index 3a4fd45147..e0c4bee893 100644
> --- a/src/test/regress/expected/create_function_3.out
> +++ b/src/test/regress/expected/create_function_3.out
> @@ -403,7 +403,7 @@ SELECT pg_get_functiondef('functest_S_13'::regproc);
>LANGUAGE sql+
> BEGIN ATOMIC +
>SELECT 1; +
> - SELECT false AS bool; +
> + SELECT false; +
> END +
Hmm, interesting side-effect: we no longer assign a column name in this
case so it remains "?column?", just like it happens for other datatypes.
This seems okay to me. (This is also what causes the changes in the
isolationtester expected output.)
--
Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/
"Ni aún el genio muy grande llegaría muy lejos
si tuviera que sacarlo todo de su propio interior" (Goethe)
Re: Add Boolean node
Ashutosh Bapat writes: > That looks like a good change. I wonder what motivates that now? Why > wasn't it added when the usages grew? You'd have to find some of the original Berkeley people to get an answer for that. Possibly it's got something to do with the fact that C didn't have a separate bool type back then ... or, going even further back, that LISP didn't either. In any case, it seems like a plausible improvement now. Didn't really read the patch in any detail, but I did have one idea: I think that the different things-that-used-to-be-Value-nodes ought to use different field names, say ival, rval, bval, sval not just "val". That makes it more likely that you'd catch any code that is doing the wrong thing and not going through one of the access macros. regards, tom lane
Re: Add Boolean node
That looks like a good change. I wonder what motivates that now? Why wasn't it added when the usages grew? Are there more Boolean usages planned? I ask because this code change will affect ability to automatically cherry-pick some of the patches. defGetBoolean() - please update the comment in the default to case to mention defGetString along with opt_boolean_or_string production. Reading the existing code in that function, one would wonder why to use true and false over say on and off. But true/false seems a natural choice. So that's fine. defGetBoolean() and nodeRead() could use a common function to parse a boolean string. The code in nodeRead() seems to assume that any string starting with "t" will represent value true. Is that right? We are using literal constants "true"/"false" at many places. This patch adds another one. I am wondering whether it makes sense to add #define TRUE_STR, FALSE_STR and use it everywhere for consistency and correctness. For the sake of consistency (again :)), we should have a function to return string representation of a Boolean node and use it in both defGetString and _outBoolean(). Are the expected output changes like below necessary? Might affect backward compatibility for applications. -bool - -t +?column? + +t On Mon, Dec 27, 2021 at 2:32 PM Peter Eisentraut wrote: > > > This patch adds a new node type Boolean, to go alongside the "value" > nodes Integer, Float, String, etc. This seems appropriate given that > Boolean values are a fundamental part of the system and are used a lot. > > Before, SQL-level Boolean constants were represented by a string with > a cast, and internal Boolean values in DDL commands were usually > represented by Integer nodes. This takes the place of both of these > uses, making the intent clearer and having some amount of type safety. -- Best Wishes, Ashutosh Bapat
Re: Add Boolean node
po 27. 12. 2021 v 13:05 odesílatel Sascha Kuhl napsal: > > > Pavel Stehule schrieb am Mo., 27. Dez. 2021, > 12:28: > >> >> >> po 27. 12. 2021 v 12:23 odesílatel Sascha Kuhl >> napsal: >> >>> >>> >>> Sascha Kuhl schrieb am Mo., 27. Dez. 2021, >>> 12:13: >>> Pavel Stehule schrieb am Mo., 27. Dez. 2021, 11:49: > Hi > > po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl > napsal: > >> You think, all values are valid. Is a higher german order valid for >> Turkey, that only know baskets, as a Form of order. For me not all forms >> of >> all are valid for all. You cannot Export or Import food that You dislike, >> because it would hurt you. Do you have dishes that you dislike? Is all >> valid for you and your culture. >> >> It is ok that this is an internal feature, that is not cultural >> dependent. Iwanted to give you my Interpretation of this Feature. It is >> ok >> It doesn't fit 😉 >> > > Please, don't use top posting mode in this mailing list > https://en.wikipedia.org/wiki/Posting_style#Top-posting > I will read and learn on that. Thanks for the hint. > This is an internal feature - Node structures are not visible from SQL > level. And internal features will be faster and less complex, if we don't > need to implement cultural dependency there. So False is just only false, > and not "false" or "lez" or "nepravda" or "Marchen" any other. > > On a custom level it is a different situation. Although I am not sure > if it is a good idea to implement local dependency for boolean type. In > Czech language we have two related words for "false" - "lez" and > "nepravda". And nothing is used in IT. But we use Czech (German) format > date (and everywhere in code ISO format shou lld be preferred), and we use > czech sorting. In internal things less complexity is better (higher > complexity means lower safety) . On a custom level, anybody can do what > they like. > >>> If you See databases as a tree, buche like books, the stem is internal, >>> less complexity, strong and safe. The custom level are the bows and leafs. >>> Ever leaf gets the ingredients it likes, but all are of the same type. >>> >> >> again - Node type is not equal to data type. >> > > Did you know that different culture have different trees. You read that. > The Chinese Bonsai reflects Chinese Société, as well as the german buche > reflects Verwaltung > > Thanks for the separation of node and data. If you consider keys, ie. > Indexes trees, keys and nodes can be easily the same, in a simulation. > Thanks for your view. > look at Postgres source code , please. https://github.com/postgres/postgres/tree/master/src/backend/nodes. In this case nodes have no relation to the index's tree. Regards Pavel >> Regards >> >> Pavel >> >>
Re: Add Boolean node
Pavel Stehule schrieb am Mo., 27. Dez. 2021, 12:28: > > > po 27. 12. 2021 v 12:23 odesílatel Sascha Kuhl > napsal: > >> >> >> Sascha Kuhl schrieb am Mo., 27. Dez. 2021, 12:13: >> >>> >>> >>> Pavel Stehule schrieb am Mo., 27. Dez. 2021, >>> 11:49: >>> Hi po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl napsal: > You think, all values are valid. Is a higher german order valid for > Turkey, that only know baskets, as a Form of order. For me not all forms > of > all are valid for all. You cannot Export or Import food that You dislike, > because it would hurt you. Do you have dishes that you dislike? Is all > valid for you and your culture. > > It is ok that this is an internal feature, that is not cultural > dependent. Iwanted to give you my Interpretation of this Feature. It is ok > It doesn't fit 😉 > Please, don't use top posting mode in this mailing list https://en.wikipedia.org/wiki/Posting_style#Top-posting >>> >>> I will read and learn on that. Thanks for the hint. >>> >>> This is an internal feature - Node structures are not visible from SQL level. And internal features will be faster and less complex, if we don't need to implement cultural dependency there. So False is just only false, and not "false" or "lez" or "nepravda" or "Marchen" any other. On a custom level it is a different situation. Although I am not sure if it is a good idea to implement local dependency for boolean type. In Czech language we have two related words for "false" - "lez" and "nepravda". And nothing is used in IT. But we use Czech (German) format date (and everywhere in code ISO format shou lld be preferred), and we use czech sorting. In internal things less complexity is better (higher complexity means lower safety) . On a custom level, anybody can do what they like. >>> >> If you See databases as a tree, buche like books, the stem is internal, >> less complexity, strong and safe. The custom level are the bows and leafs. >> Ever leaf gets the ingredients it likes, but all are of the same type. >> > > again - Node type is not equal to data type. > Did you know that different culture have different trees. You read that. The Chinese Bonsai reflects Chinese Société, as well as the german buche reflects Verwaltung Thanks for the separation of node and data. If you consider keys, ie. Indexes trees, keys and nodes can be easily the same, in a simulation. Thanks for your view. > > Regards > > Pavel > >
Re: Add Boolean node
po 27. 12. 2021 v 12:23 odesílatel Sascha Kuhl napsal: > > > Sascha Kuhl schrieb am Mo., 27. Dez. 2021, 12:13: > >> >> >> Pavel Stehule schrieb am Mo., 27. Dez. 2021, >> 11:49: >> >>> Hi >>> >>> po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl >>> napsal: >>> You think, all values are valid. Is a higher german order valid for Turkey, that only know baskets, as a Form of order. For me not all forms of all are valid for all. You cannot Export or Import food that You dislike, because it would hurt you. Do you have dishes that you dislike? Is all valid for you and your culture. It is ok that this is an internal feature, that is not cultural dependent. Iwanted to give you my Interpretation of this Feature. It is ok It doesn't fit 😉 >>> >>> Please, don't use top posting mode in this mailing list >>> https://en.wikipedia.org/wiki/Posting_style#Top-posting >>> >> >> I will read and learn on that. Thanks for the hint. >> >> >>> This is an internal feature - Node structures are not visible from SQL >>> level. And internal features will be faster and less complex, if we don't >>> need to implement cultural dependency there. So False is just only false, >>> and not "false" or "lez" or "nepravda" or "Marchen" any other. >>> >>> On a custom level it is a different situation. Although I am not sure if >>> it is a good idea to implement local dependency for boolean type. In Czech >>> language we have two related words for "false" - "lez" and "nepravda". And >>> nothing is used in IT. But we use Czech (German) format date (and >>> everywhere in code ISO format shou lld be preferred), and we use czech >>> sorting. In internal things less complexity is better (higher complexity >>> means lower safety) . On a custom level, anybody can do what they like. >>> >> > If you See databases as a tree, buche like books, the stem is internal, > less complexity, strong and safe. The custom level are the bows and leafs. > Ever leaf gets the ingredients it likes, but all are of the same type. > again - Node type is not equal to data type. Regards Pavel
Re: Add Boolean node
Sascha Kuhl schrieb am Mo., 27. Dez. 2021, 12:13: > > > Pavel Stehule schrieb am Mo., 27. Dez. 2021, > 11:49: > >> Hi >> >> po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl >> napsal: >> >>> You think, all values are valid. Is a higher german order valid for >>> Turkey, that only know baskets, as a Form of order. For me not all forms of >>> all are valid for all. You cannot Export or Import food that You dislike, >>> because it would hurt you. Do you have dishes that you dislike? Is all >>> valid for you and your culture. >>> >>> It is ok that this is an internal feature, that is not cultural >>> dependent. Iwanted to give you my Interpretation of this Feature. It is ok >>> It doesn't fit 😉 >>> >> >> Please, don't use top posting mode in this mailing list >> https://en.wikipedia.org/wiki/Posting_style#Top-posting >> > > I will read and learn on that. Thanks for the hint. > > >> This is an internal feature - Node structures are not visible from SQL >> level. And internal features will be faster and less complex, if we don't >> need to implement cultural dependency there. So False is just only false, >> and not "false" or "lez" or "nepravda" or "Marchen" any other. >> >> On a custom level it is a different situation. Although I am not sure if >> it is a good idea to implement local dependency for boolean type. In Czech >> language we have two related words for "false" - "lez" and "nepravda". And >> nothing is used in IT. But we use Czech (German) format date (and >> everywhere in code ISO format shou lld be preferred), and we use czech >> sorting. In internal things less complexity is better (higher complexity >> means lower safety) . On a custom level, anybody can do what they like. >> > If you See databases as a tree, buche like books, the stem is internal, less complexity, strong and safe. The custom level are the bows and leafs. Ever leaf gets the ingredients it likes, but all are of the same type. >> > > I agree on that from a german point of view. This is great structure on a > first guess. > > >> Regards >> >> Pavel >> >> >>> >>> Pavel Stehule schrieb am Mo., 27. Dez. 2021, >>> 11:15: >>> po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl napsal: > Can that boolean node be cultural dependent validation for the value? > By the developer? By all? > why? The boolean node is not a boolean type. This is an internal feature. There should not be any cultural dependency Regards Pavel > Pavel Stehule schrieb am Mo., 27. Dez. > 2021, 10:09: > >> >> >> po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut < >> peter.eisentr...@enterprisedb.com> napsal: >> >>> >>> This patch adds a new node type Boolean, to go alongside the "value" >>> nodes Integer, Float, String, etc. This seems appropriate given >>> that >>> Boolean values are a fundamental part of the system and are used a >>> lot. >>> >>> Before, SQL-level Boolean constants were represented by a string with >>> a cast, and internal Boolean values in DDL commands were usually >>> represented by Integer nodes. This takes the place of both of these >>> uses, making the intent clearer and having some amount of type >>> safety. >> >> >> +1 >> >> Regards >> >> Pavel >> >>
Re: Add Boolean node
Pavel Stehule schrieb am Mo., 27. Dez. 2021, 11:49: > Hi > > po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl > napsal: > >> You think, all values are valid. Is a higher german order valid for >> Turkey, that only know baskets, as a Form of order. For me not all forms of >> all are valid for all. You cannot Export or Import food that You dislike, >> because it would hurt you. Do you have dishes that you dislike? Is all >> valid for you and your culture. >> >> It is ok that this is an internal feature, that is not cultural >> dependent. Iwanted to give you my Interpretation of this Feature. It is ok >> It doesn't fit 😉 >> > > Please, don't use top posting mode in this mailing list > https://en.wikipedia.org/wiki/Posting_style#Top-posting > I will read and learn on that. Thanks for the hint. > This is an internal feature - Node structures are not visible from SQL > level. And internal features will be faster and less complex, if we don't > need to implement cultural dependency there. So False is just only false, > and not "false" or "lez" or "nepravda" or "Marchen" any other. > > On a custom level it is a different situation. Although I am not sure if > it is a good idea to implement local dependency for boolean type. In Czech > language we have two related words for "false" - "lez" and "nepravda". And > nothing is used in IT. But we use Czech (German) format date (and > everywhere in code ISO format should be preferred), and we use czech > sorting. In internal things less complexity is better (higher complexity > means lower safety) . On a custom level, anybody can do what they like. > I agree on that from a german point of view. This is great structure on a first guess. > Regards > > Pavel > > >> >> Pavel Stehule schrieb am Mo., 27. Dez. 2021, >> 11:15: >> >>> >>> >>> po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl >>> napsal: >>> Can that boolean node be cultural dependent validation for the value? By the developer? By all? >>> >>> why? >>> >>> The boolean node is not a boolean type. >>> >>> This is an internal feature. There should not be any cultural dependency >>> >>> Regards >>> >>> Pavel >>> >>> Pavel Stehule schrieb am Mo., 27. Dez. 2021, 10:09: > > > po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut < > peter.eisentr...@enterprisedb.com> napsal: > >> >> This patch adds a new node type Boolean, to go alongside the "value" >> nodes Integer, Float, String, etc. This seems appropriate given that >> Boolean values are a fundamental part of the system and are used a >> lot. >> >> Before, SQL-level Boolean constants were represented by a string with >> a cast, and internal Boolean values in DDL commands were usually >> represented by Integer nodes. This takes the place of both of these >> uses, making the intent clearer and having some amount of type safety. > > > +1 > > Regards > > Pavel > >
Re: Add Boolean node
Hi po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl napsal: > You think, all values are valid. Is a higher german order valid for > Turkey, that only know baskets, as a Form of order. For me not all forms of > all are valid for all. You cannot Export or Import food that You dislike, > because it would hurt you. Do you have dishes that you dislike? Is all > valid for you and your culture. > > It is ok that this is an internal feature, that is not cultural dependent. > Iwanted to give you my Interpretation of this Feature. It is ok It doesn't > fit 😉 > Please, don't use top posting mode in this mailing list https://en.wikipedia.org/wiki/Posting_style#Top-posting This is an internal feature - Node structures are not visible from SQL level. And internal features will be faster and less complex, if we don't need to implement cultural dependency there. So False is just only false, and not "false" or "lez" or "nepravda" or "Marchen" any other. On a custom level it is a different situation. Although I am not sure if it is a good idea to implement local dependency for boolean type. In Czech language we have two related words for "false" - "lez" and "nepravda". And nothing is used in IT. But we use Czech (German) format date (and everywhere in code ISO format should be preferred), and we use czech sorting. In internal things less complexity is better (higher complexity means lower safety) . On a custom level, anybody can do what they like. Regards Pavel > > Pavel Stehule schrieb am Mo., 27. Dez. 2021, > 11:15: > >> >> >> po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl >> napsal: >> >>> Can that boolean node be cultural dependent validation for the value? By >>> the developer? By all? >>> >> >> why? >> >> The boolean node is not a boolean type. >> >> This is an internal feature. There should not be any cultural dependency >> >> Regards >> >> Pavel >> >> >>> Pavel Stehule schrieb am Mo., 27. Dez. 2021, >>> 10:09: >>> po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut < peter.eisentr...@enterprisedb.com> napsal: > > This patch adds a new node type Boolean, to go alongside the "value" > nodes Integer, Float, String, etc. This seems appropriate given that > Boolean values are a fundamental part of the system and are used a lot. > > Before, SQL-level Boolean constants were represented by a string with > a cast, and internal Boolean values in DDL commands were usually > represented by Integer nodes. This takes the place of both of these > uses, making the intent clearer and having some amount of type safety. +1 Regards Pavel
Re: Add Boolean node
You think, all values are valid. Is a higher german order valid for Turkey, that only know baskets, as a Form of order. For me not all forms of all are valid for all. You cannot Export or Import food that You dislike, because it would hurt you. Do you have dishes that you dislike? Is all valid for you and your culture. It is ok that this is an internal feature, that is not cultural dependent. Iwanted to give you my Interpretation of this Feature. It is ok It doesn't fit 😉 Pavel Stehule schrieb am Mo., 27. Dez. 2021, 11:15: > > > po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl > napsal: > >> Can that boolean node be cultural dependent validation for the value? By >> the developer? By all? >> > > why? > > The boolean node is not a boolean type. > > This is an internal feature. There should not be any cultural dependency > > Regards > > Pavel > > >> Pavel Stehule schrieb am Mo., 27. Dez. 2021, >> 10:09: >> >>> >>> >>> po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut < >>> peter.eisentr...@enterprisedb.com> napsal: >>> This patch adds a new node type Boolean, to go alongside the "value" nodes Integer, Float, String, etc. This seems appropriate given that Boolean values are a fundamental part of the system and are used a lot. Before, SQL-level Boolean constants were represented by a string with a cast, and internal Boolean values in DDL commands were usually represented by Integer nodes. This takes the place of both of these uses, making the intent clearer and having some amount of type safety. >>> >>> >>> +1 >>> >>> Regards >>> >>> Pavel >>> >>>
Re: Add Boolean node
On Mon, Dec 27, 2021 at 5:09 PM Pavel Stehule wrote: > > po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut > napsal: >> >> This patch adds a new node type Boolean, to go alongside the "value" >> nodes Integer, Float, String, etc. This seems appropriate given that >> Boolean values are a fundamental part of the system and are used a lot. >> >> Before, SQL-level Boolean constants were represented by a string with >> a cast, and internal Boolean values in DDL commands were usually >> represented by Integer nodes. This takes the place of both of these >> uses, making the intent clearer and having some amount of type safety. > > +1 +1 too, looks like a good improvement. The patch looks good to me, although it's missing comment updates for at least nodeTokenType() and nodeRead().
Re: Add Boolean node
po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl napsal: > Can that boolean node be cultural dependent validation for the value? By > the developer? By all? > why? The boolean node is not a boolean type. This is an internal feature. There should not be any cultural dependency Regards Pavel > Pavel Stehule schrieb am Mo., 27. Dez. 2021, > 10:09: > >> >> >> po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut < >> peter.eisentr...@enterprisedb.com> napsal: >> >>> >>> This patch adds a new node type Boolean, to go alongside the "value" >>> nodes Integer, Float, String, etc. This seems appropriate given that >>> Boolean values are a fundamental part of the system and are used a lot. >>> >>> Before, SQL-level Boolean constants were represented by a string with >>> a cast, and internal Boolean values in DDL commands were usually >>> represented by Integer nodes. This takes the place of both of these >>> uses, making the intent clearer and having some amount of type safety. >> >> >> +1 >> >> Regards >> >> Pavel >> >>
Re: Add Boolean node
Can that boolean node be cultural dependent validation for the value? By the developer? By all? Pavel Stehule schrieb am Mo., 27. Dez. 2021, 10:09: > > > po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut < > peter.eisentr...@enterprisedb.com> napsal: > >> >> This patch adds a new node type Boolean, to go alongside the "value" >> nodes Integer, Float, String, etc. This seems appropriate given that >> Boolean values are a fundamental part of the system and are used a lot. >> >> Before, SQL-level Boolean constants were represented by a string with >> a cast, and internal Boolean values in DDL commands were usually >> represented by Integer nodes. This takes the place of both of these >> uses, making the intent clearer and having some amount of type safety. > > > +1 > > Regards > > Pavel > >
Re: Add Boolean node
po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut < peter.eisentr...@enterprisedb.com> napsal: > > This patch adds a new node type Boolean, to go alongside the "value" > nodes Integer, Float, String, etc. This seems appropriate given that > Boolean values are a fundamental part of the system and are used a lot. > > Before, SQL-level Boolean constants were represented by a string with > a cast, and internal Boolean values in DDL commands were usually > represented by Integer nodes. This takes the place of both of these > uses, making the intent clearer and having some amount of type safety. +1 Regards Pavel
Add Boolean node
This patch adds a new node type Boolean, to go alongside the "value"
nodes Integer, Float, String, etc. This seems appropriate given that
Boolean values are a fundamental part of the system and are used a lot.
Before, SQL-level Boolean constants were represented by a string with
a cast, and internal Boolean values in DDL commands were usually
represented by Integer nodes. This takes the place of both of these
uses, making the intent clearer and having some amount of type safety.From 4e1ef56b5443fa11d981eb6e407dfc7c244dc60e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 27 Dec 2021 09:52:05 +0100
Subject: [PATCH v1] Add Boolean node
Before, SQL-level boolean constants were represented by a string with
a cast, and internal Boolean values in DDL commands were usually
represented by Integer nodes. This takes the place of both of these
uses, making the intent clearer and having some amount of type safety.
---
contrib/postgres_fdw/postgres_fdw.c | 32 +++---
src/backend/commands/define.c | 4 +
src/backend/commands/functioncmds.c | 14 +--
src/backend/commands/sequence.c | 4 +-
src/backend/commands/tsearchcmds.c| 9 ++
src/backend/commands/user.c | 28 +++---
src/backend/nodes/copyfuncs.c | 16 +++
src/backend/nodes/equalfuncs.c| 11 +++
src/backend/nodes/nodeFuncs.c | 1 +
src/backend/nodes/outfuncs.c | 8 ++
src/backend/nodes/read.c | 5 +
src/backend/nodes/value.c | 12 +++
src/backend/parser/gram.y | 98 +--
src/backend/parser/parse_node.c | 8 ++
src/backend/replication/repl_gram.y | 14 +--
src/include/nodes/nodes.h | 1 +
src/include/nodes/parsenodes.h| 1 +
src/include/nodes/value.h | 8 ++
src/test/isolation/expected/ri-trigger.out| 60 ++--
.../regress/expected/create_function_3.out| 2 +-
20 files changed, 210 insertions(+), 126 deletions(-)
diff --git a/contrib/postgres_fdw/postgres_fdw.c
b/contrib/postgres_fdw/postgres_fdw.c
index fa9a099f13..4d61d88d7b 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -103,7 +103,7 @@ enum FdwModifyPrivateIndex
FdwModifyPrivateTargetAttnums,
/* Length till the end of VALUES clause (as an Integer node) */
FdwModifyPrivateLen,
- /* has-returning flag (as an Integer node) */
+ /* has-returning flag (as a Boolean node) */
FdwModifyPrivateHasReturning,
/* Integer list of attribute numbers retrieved by RETURNING */
FdwModifyPrivateRetrievedAttrs
@@ -122,11 +122,11 @@ enum FdwDirectModifyPrivateIndex
{
/* SQL statement to execute remotely (as a String node) */
FdwDirectModifyPrivateUpdateSql,
- /* has-returning flag (as an Integer node) */
+ /* has-returning flag (as a Boolean node) */
FdwDirectModifyPrivateHasReturning,
/* Integer list of attribute numbers retrieved by RETURNING */
FdwDirectModifyPrivateRetrievedAttrs,
- /* set-processed flag (as an Integer node) */
+ /* set-processed flag (as a Boolean node) */
FdwDirectModifyPrivateSetProcessed
};
@@ -280,9 +280,9 @@ typedef struct PgFdwAnalyzeState
*/
enum FdwPathPrivateIndex
{
- /* has-final-sort flag (as an Integer node) */
+ /* has-final-sort flag (as a Boolean node) */
FdwPathPrivateHasFinalSort,
- /* has-limit flag (as an Integer node) */
+ /* has-limit flag (as a Boolean node) */
FdwPathPrivateHasLimit
};
@@ -1245,9 +1245,9 @@ postgresGetForeignPlan(PlannerInfo *root,
*/
if (best_path->fdw_private)
{
- has_final_sort = intVal(list_nth(best_path->fdw_private,
+ has_final_sort = boolVal(list_nth(best_path->fdw_private,
FdwPathPrivateHasFinalSort));
- has_limit = intVal(list_nth(best_path->fdw_private,
+ has_limit = boolVal(list_nth(best_path->fdw_private,
FdwPathPrivateHasLimit));
}
@@ -1879,7 +1879,7 @@ postgresPlanForeignModify(PlannerInfo *root,
return list_make5(makeString(sql.data),
targetAttrs,
makeInteger(values_end_len),
- makeInteger((retrieved_attrs != NIL)),
+ makeBoolean((retrieved_attrs != NIL)),
retrieved_attrs);
}
@@ -1916,7 +1916,7 @@ postgresBeginForeignModify(
