Re: Use boolean array for nulls parameters

2021-01-19 Thread japin


On Tue, 19 Jan 2021 at 23:45, Tom Lane  wrote:
> japin  writes:
>> When I review the [1], I find that the tuple's nulls array use char type.
>> However there are many places use boolean array to repsent the nulls array,
>> so I think we can replace the char type nulls array to boolean type.  This
>> change will break the SPI_xxx API, I'm not sure whether this chagnges cause
>> other problems or not.  Any thought?
>
> We have always considered that changing the APIs of published SPI
> interfaces is a non-starter.  The entire reason those calls still
> exist at all is for the benefit of third-party extensions.
>

Thanks for your clarify.  I agree that we should keep the APIs stable, maybe we
can modify this in someday when the APIs must be changed.

-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.




Re: Use boolean array for nulls parameters

2021-01-19 Thread Tom Lane
japin  writes:
> When I review the [1], I find that the tuple's nulls array use char type.
> However there are many places use boolean array to repsent the nulls array,
> so I think we can replace the char type nulls array to boolean type.  This
> change will break the SPI_xxx API, I'm not sure whether this chagnges cause
> other problems or not.  Any thought?

We have always considered that changing the APIs of published SPI
interfaces is a non-starter.  The entire reason those calls still
exist at all is for the benefit of third-party extensions.

regards, tom lane




Re: Use boolean array for nulls parameters

2021-01-19 Thread Hamid Akhtar
I personally don't see any benefit in this change. The focus shouldn't be
on fixing things that aren't broken. Perhaps, there is more value in using
bitmap data type to keep track of NULL values, which is typical storage vs
performance debate, and IMHO, it's better to err on using slightly more
storage for much better performance. IIRC, the bitmap idea has previously
discussed been rejected too.

On Tue, Jan 19, 2021 at 7:07 PM japin  wrote:

>
> Hi,
>
> When I review the [1], I find that the tuple's nulls array use char type.
> However there are many places use boolean array to repsent the nulls array,
> so I think we can replace the char type nulls array to boolean type.  This
> change will break the SPI_xxx API, I'm not sure whether this chagnges cause
> other problems or not.  Any thought?
>
> [1] -
> https://www.postgresql.org/message-id/flat/ca+hiwqgkfjfydeq5vhph6eqpkjsbfpddy+j-kxyfepqedts...@mail.gmail.com
>
> --
> Regrads,
> Japin Li.
> ChengDu WenWu Information Technology Co.,Ltd.
>
>

-- 
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL:+923335449950  EMAIL: mailto:hamid.akh...@highgo.ca
SKYPE: engineeredvirus


Use boolean array for nulls parameters

2021-01-19 Thread japin

Hi,

When I review the [1], I find that the tuple's nulls array use char type.
However there are many places use boolean array to repsent the nulls array,
so I think we can replace the char type nulls array to boolean type.  This
change will break the SPI_xxx API, I'm not sure whether this chagnges cause
other problems or not.  Any thought?

[1] - 
https://www.postgresql.org/message-id/flat/ca+hiwqgkfjfydeq5vhph6eqpkjsbfpddy+j-kxyfepqedts...@mail.gmail.com

-- 
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

>From 7496f20ccfda7687333db9e5c43227ee30e4eda9 Mon Sep 17 00:00:00 2001
From: Japin Li 
Date: Tue, 19 Jan 2021 15:51:44 +0800
Subject: [PATCH v1] Replace the char to bool array for null parameters

---
 doc/src/sgml/spi.sgml   | 60 -
 src/backend/executor/spi.c  | 18 -
 src/backend/utils/adt/ri_triggers.c |  8 ++--
 src/backend/utils/adt/ruleutils.c   | 10 ++---
 src/include/executor/spi.h  | 12 +++---
 src/pl/plperl/plperl.c  |  6 +--
 src/pl/plpython/plpy_cursorobject.c |  6 +--
 src/test/regress/regress.c  | 12 +++---
 8 files changed, 61 insertions(+), 71 deletions(-)

diff --git a/doc/src/sgml/spi.sgml b/doc/src/sgml/spi.sgml
index f5e0a35da0..d3cc405c42 100644
--- a/doc/src/sgml/spi.sgml
+++ b/doc/src/sgml/spi.sgml
@@ -649,7 +649,7 @@ int SPI_exec(const char * command, long count<
 
 int SPI_execute_with_args(const char *command,
   int nargs, Oid *argtypes,
-  Datum *values, const char *nulls,
+  Datum *values, const bool *nulls,
   bool read_only, long count)
 
  
@@ -728,7 +728,7 @@ int SPI_execute_with_args(const char *command,

 

-const char * nulls
+const bool * nulls
 
  
   an array of length nargs, describing which
@@ -739,12 +739,10 @@ int SPI_execute_with_args(const char *command,
   If nulls is NULL then
   SPI_execute_with_args assumes that no parameters
   are null.  Otherwise, each entry of the nulls
-  array should be ' ' if the corresponding parameter
-  value is non-null, or 'n' if the corresponding parameter
+  array should be false if the corresponding parameter
+  value is non-null, or true if the corresponding parameter
   value is null.  (In the latter case, the actual value in the
-  corresponding values entry doesn't matter.)  Note
-  that nulls is not a text string, just an array:
-  it does not need a '\0' terminator.
+  corresponding values entry doesn't matter.)
  
 

@@ -1601,7 +1599,7 @@ bool SPI_is_cursor_plan(SPIPlanPtr plan)
 
  
 
-int SPI_execute_plan(SPIPlanPtr plan, Datum * values, const char * nulls,
+int SPI_execute_plan(SPIPlanPtr plan, Datum * values, const bool * nulls,
  bool read_only, long count)
 
  
@@ -1642,7 +1640,7 @@ int SPI_execute_plan(SPIPlanPtr plan, Datum * 

 

-const char * nulls
+const bool * nulls
 
  
   An array describing which parameters are null.  Must have same length as
@@ -1653,12 +1651,10 @@ int SPI_execute_plan(SPIPlanPtr plan, Datum * 
   If nulls is NULL then
   SPI_execute_plan assumes that no parameters
   are null.  Otherwise, each entry of the nulls
-  array should be ' ' if the corresponding parameter
-  value is non-null, or 'n' if the corresponding parameter
+  array should be false if the corresponding parameter
+  value is non-null, or true if the corresponding parameter
   value is null.  (In the latter case, the actual value in the
-  corresponding values entry doesn't matter.)  Note
-  that nulls is not a text string, just an array:
-  it does not need a '\0' terminator.
+  corresponding values entry doesn't matter.)
  
 

@@ -1946,7 +1942,7 @@ int SPI_execute_plan_with_receiver(SPIPlanPtr plan,
 
  
 
-int SPI_execp(SPIPlanPtr plan, Datum * values, const char * nulls, long count)
+int SPI_execp(SPIPlanPtr plan, Datum * values, const bool * nulls, long count)
 
  
 
@@ -1985,7 +1981,7 @@ int SPI_execp(SPIPlanPtr plan, Datum * values<

 

-const char * nulls
+const bool * nulls
 
  
   An array describing which parameters are null.  Must have same length as
@@ -1996,12 +1992,10 @@ int SPI_execp(SPIPlanPtr plan, Datum * values<
   If nulls is NULL then
   SPI_execp assumes that no parameters
   are null.  Otherwise, each entry of the nulls
-  array should be ' ' if the corresponding parameter
-  value is non-null, or 'n' if the corresponding parameter
+  array should be false if the corresponding parameter
+  value is non-null, or true if the corresponding parameter
   value is null.  (In the latter case, the actual value in the
-  corresponding values entry doesn't matter.)  Note
-  that nulls is not a text string, just an array:
-  it does not need a '\0