Add Boolean node

Started by Peter Eisentrautover 4 years ago27 messageshackers
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

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.

Attachments:

v1-0001-Add-Boolean-node.patchtext/plain; charset=UTF-8; name=v1-0001-Add-Boolean-node.patchDownload+210-117
#2Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#1)
Re: Add Boolean node

po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut <
peter.eisentraut@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

#3Sascha Kuhl
yogidabanli@gmail.com
In reply to: Pavel Stehule (#2)
Re: Add Boolean node

Can that boolean node be cultural dependent validation for the value? By
the developer? By all?

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
10:09:

Show quoted text

po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut <
peter.eisentraut@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

#4Pavel Stehule
pavel.stehule@gmail.com
In reply to: Sascha Kuhl (#3)
Re: Add Boolean node

po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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

Show quoted text

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
10:09:

po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut <
peter.eisentraut@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

#5Julien Rouhaud
rjuju123@gmail.com
In reply to: Pavel Stehule (#2)
Re: Add Boolean node

On Mon, Dec 27, 2021 at 5:09 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:

po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut <peter.eisentraut@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

+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().

#6Sascha Kuhl
yogidabanli@gmail.com
In reply to: Pavel Stehule (#4)
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 <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
11:15:

Show quoted text

po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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 <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
10:09:

po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut <
peter.eisentraut@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

#7Pavel Stehule
pavel.stehule@gmail.com
In reply to: Sascha Kuhl (#6)
Re: Add Boolean node

Hi

po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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

Show quoted text

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
11:15:

po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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 <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
10:09:

po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut <
peter.eisentraut@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

#8Sascha Kuhl
yogidabanli@gmail.com
In reply to: Pavel Stehule (#7)
Re: Add Boolean node

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
11:49:

Hi

po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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.

Show quoted text

Regards

Pavel

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
11:15:

po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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 <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
10:09:

po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut <
peter.eisentraut@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

#9Sascha Kuhl
yogidabanli@gmail.com
In reply to: Sascha Kuhl (#8)
Re: Add Boolean node

Sascha Kuhl <yogidabanli@gmail.com> schrieb am Mo., 27. Dez. 2021, 12:13:

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
11:49:

Hi

po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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.

Show quoted text

I agree on that from a german point of view. This is great structure on a
first guess.

Regards

Pavel

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
11:15:

po 27. 12. 2021 v 11:08 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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 <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez.
2021, 10:09:

po 27. 12. 2021 v 10:02 odesílatel Peter Eisentraut <
peter.eisentraut@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

#10Pavel Stehule
pavel.stehule@gmail.com
In reply to: Sascha Kuhl (#9)
Re: Add Boolean node

po 27. 12. 2021 v 12:23 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
napsal:

Sascha Kuhl <yogidabanli@gmail.com> schrieb am Mo., 27. Dez. 2021, 12:13:

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
11:49:

Hi

po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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

#11Sascha Kuhl
yogidabanli@gmail.com
In reply to: Pavel Stehule (#10)
Re: Add Boolean node

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
12:28:

po 27. 12. 2021 v 12:23 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
napsal:

Sascha Kuhl <yogidabanli@gmail.com> schrieb am Mo., 27. Dez. 2021, 12:13:

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
11:49:

Hi

po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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.

Show quoted text

Regards

Pavel

#12Pavel Stehule
pavel.stehule@gmail.com
In reply to: Sascha Kuhl (#11)
Re: Add Boolean node

po 27. 12. 2021 v 13:05 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
napsal:

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
12:28:

po 27. 12. 2021 v 12:23 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
napsal:

Sascha Kuhl <yogidabanli@gmail.com> schrieb am Mo., 27. Dez. 2021,
12:13:

Pavel Stehule <pavel.stehule@gmail.com> schrieb am Mo., 27. Dez. 2021,
11:49:

Hi

po 27. 12. 2021 v 11:24 odesílatel Sascha Kuhl <yogidabanli@gmail.com>
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

Show quoted text

Regards

Pavel

#13Ashutosh Bapat
ashutosh.bapat@enterprisedb.com
In reply to: Peter Eisentraut (#1)
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
<peter.eisentraut@enterprisedb.com> 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

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Ashutosh Bapat (#13)
Re: Add Boolean node

Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> 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

#15Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Eisentraut (#1)
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)

#16Zhihong Yu
zyu@yugabyte.com
In reply to: Alvaro Herrera (#15)
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

#17Peter Eisentraut
peter_e@gmx.net
In reply to: Ashutosh Bapat (#13)
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.

#18Josef Šimánek
josef.simanek@gmail.com
In reply to: Alvaro Herrera (#15)
Re: Add Boolean node

po 27. 12. 2021 v 16:10 odesílatel Alvaro Herrera
<alvherre@alvh.no-ip.org> 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.

Show quoted text

--
Á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)

#19Tom Lane
tgl@sss.pgh.pa.us
In reply to: Josef Šimánek (#18)
Re: Add Boolean node

=?UTF-8?B?Sm9zZWYgxaBpbcOhbmVr?= <josef.simanek@gmail.com> writes:

po 27. 12. 2021 v 16:10 odesílatel Alvaro Herrera
<alvherre@alvh.no-ip.org> 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

#20Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#14)
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.

#21Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#20)
#22Andres Freund
andres@anarazel.de
In reply to: Peter Eisentraut (#1)
#23Peter Eisentraut
peter_e@gmx.net
In reply to: Andres Freund (#20)
#24Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Eisentraut (#1)
#25Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Eisentraut (#24)
#26Pavel Stehule
pavel.stehule@gmail.com
In reply to: Peter Eisentraut (#25)
#27Peter Eisentraut
peter_e@gmx.net
In reply to: Pavel Stehule (#26)