RE: [Patch] PQconnectPoll() is blocked if target_session_attrs is read-write
Tsunakawa-san Thank you for your comment. I understand the sense. I don't require an explicit rule. Regards Ryo Matsumura
RE: [Patch] PQconnectPoll() is blocked if target_session_attrs is read-write
Kyotaro-san Thank you for your review. > First, this patch looks broken. I took a serious mistake. > You also need to update the corresponding documentation. I attach a new patch that includes updating of document. Regards Ryo Matasumura libpq_state_change_bugfix.ver1.1.patch Description: libpq_state_change_bugfix.ver1.1.patch
[Patch] PQconnectPoll() is blocked if target_session_attrs is read-write
Hi
# I rewrote my previous mail.
PQconnectPoll() is used as method for asynchronous using externally or
internally.
If a caller confirms a socket ready for writing or reading that is
requested by return value of previous PQconnectPoll(), next PQconnectPoll()
must not be blocked. But if the caller specifies target_session_attrs to
'read-write', PQconnectPoll() may be blocked.
Detail:
If target_session_attrs is set to read-write, PQconnectPoll() calls
PQsendQuery("SHOW transaction_read_only") althogh previous return value was
PGRES_POLLING_READING not WRITING.
In result, PQsendQuery() may be blocked in pqsecure_raw_write().
I attach a patch.
Regards
Ryo Matsumura
libpq_state_change_bugfix.ver1.0.patch
Description: libpq_state_change_bugfix.ver1.0.patch
A suspicious code in PQconnectPoll()
Hi
I find a suspicious code in libpq:PQconnectPoll(). I think it should be
fixed, but I could not produce a concrete problem.
What do you think about it?
I understand that PQconnectPoll() returns PGRES_POLLING_WRITING or
PGRES_POLLING_READ until state machine reaches terminal state(OK or BAD).
The return value indicates for users which event they should wait for
before next PQconnectPoll().
But PQconnectPoll() calls PQsendQuery("SHOW transaction_read_only")
in CONNECTION_AUTH_OK without returning PGRES_POLLING_WRITING before.
My idea is as following:
case CONNECTION_AWAITING_RESPONSE:
receive authetication OK, transit state machine to AUTH_OK and
return PGRES_POLLING_READING.
case CONNECTION_AUTH_OK:
clear any data from backend using PQisBusy(), transit to
CHECK_WRITABLE_STARTED(new state!), and return PGRES_POLLING_WRITING.
case CONNECTION_CHECK_WRITABLE_STARTED (new state!):
call PQsendQuery("SHOW transaction_read_only"), and transit to
CONNECTION_CHECK_WRITABLE, and return CONNECTION_CHECK_READING.
Regards
Ryo Matsumura
RE: Bug: ECPG: Cannot use CREATE AS EXECUTE statemnt
Meskes-san Thank you for your comment. I attach a patch. It doesn't include tests, but it passed some test(*1). Explanation about the patch: - Add a new ECPGst_exec_embedded_in_other_stmt whether EXECUTE statement has exprlist or not. This type name may not be good. It is a type for [CREATE TABLE ... AS EXECUTE ...]. But I doesn't consider about [EXPLAIN EXECUTE ...]. - If statement type is a new one, ecpglib embeds variables into query in text format at ecpg_build_params(). Even if the statement does not have exprlist, ecpglib makes exprlist and embeds into it. The list is expanded incrementally in loop of ecpg_build_params(). - ecpg_build_params() is difficult to read and insert the above logic. Therefore, I refactor it. The deitail is described in comments. (*1) The followings run expectively. exec sql create table if not exists foo (c1 int); exec sql insert into foo select generate_series(1, 20); exec sql prepare st as select * from foo where c1 % $1 = 0 and c1 % $2 = 0; exec sql execute st using :v1,:v2; exec sql execute st(:v1,:v2); exec sql create table if not exists bar (c1) as execute st(2, 3); exec sql create table if not exists bar (c1) as execute st using 2,3; exec sql create table if not exists bar (c1) as execute st using :v1,:v2; exec sql create table bar (c1) as execute st using :v1,:v2; Regards Ryo Matsumura ecpg_createas_execute.v1.0.patch Description: ecpg_createas_execute.v1.0.patch
RE: [PATCH] memory leak in ecpglib
Hi
On Mon. June. 10, 2019 at 09:54 AM Zhang, Jie
< zhangj...@cn.fujitsu.com > wrote:
>
> Memory leaks occur when the ecpg_update_declare_statement() is called the
> second time.
Certainly it is.
But I wonder if it is safe that the old cursor_name is forgotten.
Regards
Ryo Matsumura
> -Original Message-
> From: Zhang, Jie [mailto:zhangj...@cn.fujitsu.com]
> Sent: Monday, June 10, 2019 9:54 AM
> To: pgsql-hackers@lists.postgresql.org
> Cc: Zhang, Jie/张 杰
> Subject: [PATCH] memory leak in ecpglib
>
> Hi all
>
> Memory leaks occur when the ecpg_update_declare_statement() is called the
> second time.
>
> FILE:postgresql\src\interfaces\ecpg\ecpglib\prepare.c
> void
> ecpg_update_declare_statement(const char *declared_name, const char
> *cursor_name, const int lineno)
> {
> struct declared_statement *p = NULL;
>
> if (!declared_name || !cursor_name)
> return;
>
> /* Find the declared node by declared name */
> p = ecpg_find_declared_statement(declared_name);
> if (p)
> p->cursor_name = ecpg_strdup(cursor_name, lineno); ★
> }
> ecpg_strdup() returns a pointer to a null-terminated byte string, which is
> a duplicate of the string pointed to by str.
> The memory obtained is done dynamically using malloc and hence it can be freed
> using free().
>
> When the ecpg_update_declare_statement() is called for the second time,
> the memory allocated for p->cursor_name is not freed.
>
> For example:
>
> EXEC SQL BEGIN DECLARE SECTION;
> char *selectString = "SELECT * FROM foo;";
> int FooBar;
> char DooDad[17];
> EXEC SQL END DECLARE SECTION;
>
> EXEC SQL CONNECT TO postgres@localhost:5432 AS con1 USER postgres;
>
> EXEC SQL AT con1 DECLARE stmt_1 STATEMENT;
> EXEC SQL AT con1 PREPARE stmt_1 FROM :selectString;
>
> EXEC SQL AT con1 DECLARE cur_1 CURSOR FOR stmt_1; //★1 ECPGopen()
> --> ecpg_update_declare_statement()
> EXEC SQL AT con1 OPEN cur_1;
>
> EXEC SQL AT con1 DECLARE cur_2 CURSOR FOR stmt_1; //★2 ECPGopen()
> --> ecpg_update_declare_statement()
> EXEC SQL AT con1 OPEN cur_2;
> Memory leaks
>
> EXEC SQL FETCH cur_2 INTO:FooBar, :DooDad;
> EXEC SQL COMMIT;
> EXEC SQL DISCONNECT ALL;
>
>
> We should free p->cursor_name before p->cursor_name = ecpg_strdup(cursor_name,
> lineno).
> #
>
> if(p->cursor_name)
> ecpg_free(p->cursor_name);
> p->cursor_name = ecpg_strdup(cursor_name,lineno);
> #
> ##
> Here is a patch.
>
> Best Regards!
>
>
Bug: ECPG: Cannot use CREATE AS EXECUTE statemnt
Meskes-san
This thread is branched from the following.
https://www.postgresql.org/message-id/03040DFF97E6E54E88D3BFEE5F5480F74ABEADE7@G01JPEXMBYT04
> > Type1. Bugs or intentional unsupported features.
> > - EXPLAIN EXECUTE
> > - **CREATE TABLE AS with using clause**
I noticed that CREATE AS EXECUTE with using clause needs a new
implementation that all parameters in using clause must be embedded into
expr-list of EXECUTE in text-format as the following because there is
no interface of protocol for our purpose.
It spends more time for implementing. Do you have any advice?
int id = 100;
EXEC SQL CREATE TABLE test AS EXECUTE stmt using :id;
-->
PQexec("CREATE TABLE test AS EXECUTE stmt(100)");
e.g. PQexecParamas("CREATE TABLE test AS EXECUTE stmt",
{23,0},{"100",0},{3,0},NULL)
It sends the following.
To backend> Msg P
To backend> ""
To backend> "create table test as execute stmt"
:
To backend> Msg B
To backend> ""
To backend> "" ---> It means execute request "create table test as
execute stmt" with the value.
To backend (2#)> 1 But the create statement has no $x. Since the
value may be discard.
To backend (2#)> 0 In result, the following error is occurred.
To backend (2#)> 1
To backend (4#)> 3
To backend> 100
To backend (2#)> 1
To backend (2#)> 0
:
2019-06-06 07:26:35.252 UTC [1630] ERROR: wrong number of parameters for
prepared statement "stmt"
2019-06-06 07:26:35.252 UTC [1630] DETAIL: Expected 1 parameters but got 0.
2019-06-06 07:26:35.252 UTC [1630] STATEMENT: create table test2 as
execute stmt
Regards
Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Meskes-san > This looks good to me. It passes all my tests, too. > > There were two minor issues, the regression test did not run and gcc > complained about the indentation in ECPGprepare(). Both I easily fixed. Thank you so much ! > > (2) > > I found some bugs (two types). I didn't fix them and only avoid bison > > error. > > > > Type1. Bugs or intentional unsupported features. > > - EXPLAIN EXECUTE > > - CREATE TABLE AS with using clause > > ... > > Please send a patch. I'm on vacation and won't be able to spend time on > this for the next couple of weeks. I begin to fix it. It may spend a while (1 or 2 week). > > Type2. In multi-bytes encoding environment, a part of character of > > message is cut off. > > > > It may be very difficult to fix. I pretend I didn't see it for a > > while. > > ... > > Hmm, any suggestion? I think that it's better to import length_in_encoding() defined in backend/utils/mb/mbutils.c into client side. Just an idea. Regards Ryo Matsumura
RE: Patch: doc for pg_logical_emit_message()
On Thu. May. 9, 2019 at 01:48 AM Masao, Fujii wrote: > Thanks! Pushed. Thank you. Regards Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Hi Meskes-san
There are two points.
(1)
I attach a new patch. Please review it.
- Preproc replaces any prepared_name to "$0" and changes it to an
input-variable
for PREARE with typelist and EXECUTE with paramlist.
$0 is replaced in ecpg_build_params().
It's enable not to change ECPGdo interface.
- Host variables can be used in paramlist of EXECUTE.
(2)
I found some bugs (two types). I didn't fix them and only avoid bison error.
Type1. Bugs or intentional unsupported features.
- EXPLAIN EXECUTE
- CREATE TABLE AS with using clause
e.g.
EXPLAIN EXECUTE st; /* It has not been supported before.*/
ExplainableStmt:
ExecuteStmt
{
- $$ = $1;
+ $$ = $1.name;/* only work arround for bison error */
}
Type2. In multi-bytes encoding environment, a part of character of message is
cut off.
It may be very difficult to fix. I pretend I didn't see it for a while.
[ecpglib/error.c]
snprintf(sqlca->sqlerrm.sqlerrmc, sizeof(sqlca->sqlerrm.sqlerrmc), "%s on
line %d", message, line);
sqlca->sqlerrm.sqlerrml = strlen(sqlca->sqlerrm.sqlerrmc);
ecpg_log("raising sqlstate %.*s (sqlcode %ld): %s\n",
(int) sizeof(sqlca->sqlstate), sqlca->sqlstate, sqlca->sqlcode,
sqlca->sqlerrm.sqlerrmc);
Regards
Ryo Matsumura
ecpg_prepare_as_v1_2.patch
Description: ecpg_prepare_as_v1_2.patch
RE: SQL statement PREPARE does not work in ECPG
Meskes-san > This look very reasonable to me. I'm completely fine with this > restriction being placed on PREPARE FROM. Thank you. I start making a new patch. Regards Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Hi, Meskes-san I'm sorry for my long blank. I restarted. Review of previous discussion: I made a patch that makes ecpglib to send "PREPARE st(typelist) AS PreparableStmt" with PQexec(), because the type resolution is difficult. I tried to merge PREPARE FROM that uses PQprepare() to the PREPARE AS. Meskes-san pointed that there may be a problem that PREPARE FROM cannot use PQexec(). Now, I noticed a problem of the merging. Therefore, I will not change the existing implementation of PREPARE FROM. The problem is: Statement name of PREPARE FROM can include double quote, because the statement name is sent by PQprepare() directly and backend doesn't parse it. In other hand, the statement name of PREPARE AS cannot include double quote, because it is embedded into query and backend parser disallows it. This is a specification of PostgreSQL's PREPARE AS. I defined the following specifications. Please review it. * ECPG can accept any valid PREPARE AS statement. * A char-type host variable can be used as the statement name of PREPARE AS, but its value is constrained by the specification of PREPARE AS. (e.g. the name cannot include double quotation.) * The above also allows the following. It's a bit strange but there is no reason for forbidding. prepare :st(type_list) as select $1 * ECPG can accept EXECUTE statement with expression list that is allocated by both PREPARE FROM and PREPARE AS under the following constraints: - It must not include a using-clause. - The statement name must follow to the specification of PREPARE AS. Regards Ryo Matsumura
RE: Patch: doc for pg_logical_emit_message()
On Wed. Apr. 24, 2019 at 11:40 PM Masao, Fujii wrote: Thank you for the comment. I understand about REPLICATION privilege and notice my unecessary words. I update the patch. Regards Ryo Matsumura pg_logical_emit_message_doc_v1_2.patch Description: pg_logical_emit_message_doc_v1_2.patch
RE: Patch: doc for pg_logical_emit_message()
On Tue. Apr. 23, 2019 at 02:59 AM Masao, Fujii wrote: Thank you for the comment. > So I think that the patch should fix also the description for those > replication functions. Thought? I think so too. I attach a new patch. Regards Ryo Matsumura replication_functions_doc.patch Description: replication_functions_doc.patch
Patch: doc for pg_logical_emit_message()
Hi, Hackers pg_logical_emit_message() can be used by any user, but the following document says that it can be used by only superuser. > Table 9.88. Replication SQL Functions > Use of these functions is restricted to superusers. I think that pg_logicl_emit_message() should be used by any user. Therefore, I attach the document patch. Ryo Matsumura pg_logical_emit_message_doc.patch Description: pg_logical_emit_message_doc.patch
RE: Qestion about .partial WAL file
Michael-san Thank for your advice. > then a promoted standby which archives WAL segments in the same > location as the primary > if the previous primary is still running after the standby I could not come up with the combination, but I understand now. Sorry for bothering you. Regards Ryo Matsumura
Qestion about .partial WAL file
Hi, Hackers I noticed something strange. Does it cause nothing? I didn't detect anything, but feel restless. Step: - There are two standbys that connect to primary. - Kill primary and promote one standby. - Restart another standby that is reset primary_conninfo to connect new primary. I expected that the latest WAL segment file in old timeline is renamed with .partial suffix, but it's not renamed in the restarted standby. xlog.c says the following, but I didn't understand the bad situation. * the archive. It's physically present in the new file with new TLI, * but recovery won't look there when it's recovering to the older --> * timeline. On the other hand, if we archive the partial segment, and --> * the original server on that timeline is still running and archives --> * the completed version of the same segment later, it will fail. (We * used to do that in 9.4 and below, and it caused such problems). * * As a compromise, we rename the last segment with the .partial * suffix, and archive it. Archive recovery will never try to read * .partial segments, so they will normally go unused. But in the odd * PITR case, the administrator can copy them manually to the pg_wal * directory (removing the suffix). They can be useful in debugging, * too. Regards Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Meskes-san I'm sorry for my slow reply. > I agree that we have to accept a quoted statement name and your > observations are correct of course, I am merely wondering if we need > the escaped quotes in the call to the ecpg functions or the libpq > functions. The following allows to use statement name including white space not double-quoted statement name. exec sql prepare "st1 x" from "select 1"; # I don't know whether the existing ECPG allows it intentionally or not. # In honestly, I think that it's not necessary to allow it. If we also allow the statement name including white space in PREPRARE AS, we have to make backend parser to scan it as IDENT. Double-quoting is one way. There may be another way. If we want to pass double-quoted statement name to backend through libpq, preprocessor have to escape it. > > I would prefer to merge as much as possible, as I am afraid that if > > we > > do not merge the approaches, we will run into issues later. There > > was a > > reason why we added PQprepare, but I do not remember it from the > > top of > > my head. Need to check when I'm online again. > > I will also consider it. I cannot think of anything. I may notice if I try to merge. Regards Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Meskes-san Thank you for your comment. > One question though, why is the statement name always quoted? Do we > really need that? Seems to be more of a hassle than and advantage. The following can be accepted by preproc, ecpglib, libpq, and backend in previous versions. exec sql prepare "st x" from "select 1"; exec sql execute "st x"; The above was preprocessed to the following. PQprepare(conn, "\"st x\"", "select 1"); PQexec(conn, "\"st x\""); By the way, the following also can be accepted. PQexecParams(conn, "prepare \"st x\" ( int ) as select $1", 0, NULL, NULL, NULL, NULL, 0); PQexecParams(conn, "execute \"st x\"( 1 )", 0, NULL, NULL, NULL, NULL, 0); Therefore, I think that the quoting statement name is needed in PREPARE/EXECUTE case, too. > I would prefer to merge as much as possible, as I am afraid that if we > do not merge the approaches, we will run into issues later. There was a > reason why we added PQprepare, but I do not remember it from the top of > my head. Need to check when I'm online again. I will also consider it. Regards Ryo Matsumura
RE: Is PREPARE of ecpglib thread safe?
Horiguchi-san, Kuroda-san > Horiguchi-san wrote: > > A namespace of declared statement is not connection independent. > > Therefore, we must manage the namespce in global and consider about race > > condition. > > Right, and but thread independent. I was wrong. I understand that DECLARE STATEMENT should be same policy as the combination of PREPARE STATEMENT and SET CONNECTION. We should fix the current implementation of DECLARE STATEMENT. Current: t1:Thread1: exec sql at conn1 declare st1 statement; t2:Thread2: exec sql at conn2 declare st1 statement; // NG ToBe: t1:Thread1: exec sql at conn1 declare st1 statement; t2:Thread2: exec sql at conn2 declare st1 statement; // OK t3:Thread2: exec sql prepared st1 from "select 1";// OK: prepared on conn2 t4:Thread1: exec sql execute st1; // NG: not prepared t5:Thread2: exec sql execute st1; // OK: executed on conn2 t1:Thread1: exec sql at conn1 declare st1 statement; t2:Thread1: exec sql at conn2 declare st1 statement; // NG Regards Ryo Matsumura
RE: Is PREPARE of ecpglib thread safe?
Hi Horiguchi-san, Kuroda-san Horiguchi-san, thank you for your comment. I have a question. A bug of StatementCache is occurred in previous versions. Should a patch be separated? > Horiguchi-san wrote: > It seems like a local cache of server-side data, which is similar > to catcache on server side for each process. I agree. I will fix it with using pthread_setspecific like descriptor.c. > I don't think > prepared statements is in that category. A prepared statement is > bonded to a connection, not to a thread. Different threads can > execute the same prepared statement on the same connection. A namespace of declared statement is not connection independent. Therefore, we must manage the namespce in global and consider about race condition. For example, ecpglib must refer the information of (A) when ecpglib executes (B) in order to occur "double declare" error. (A) exec sql at conn1 declare st1 statement; (B) exec sql at conn2 declare st1 statement; // If ecpglib didn't reject the above, ecpglib cannot judge // which connection the followings should be executed on. exec sql prepare st1 from "select 1"; exec sql execute st1; Kuroda-san, is it right? If it's right, I will fix it with using pthread_lock. Regards Ryo Matsumura
RE: Is PREPARE of ecpglib thread safe?
Horiguchi-san Thank you for your comment. > A connection cannot be concurrently used by multiple threads so > the programmer must guard connections using mutex [1] or > friends. If it is done by a single mutex (I suppose it is > common.), there's no race condition also on the prepared > statement storage. I'm not sure it is explicitly aimed but I > suppose that there's no problem in a common usage of the library. I understand it, but current scope of StatementCache and DeclareStatementList seems not to be limitted within each connection, isn't it? Therefore, I thought the operation on them must be thread safe. For example, scope of DescriptorList in descriptor.c is within thread (not connection) by using pthread_getspecific/ pthread_setspecific(). Regards Ryo Matsumura
Is PREPARE of ecpglib thread safe?
Hi I'm afraid that PREPARE of ecpglib is not thread safe. The following global variables are modified without any locking system. Is it unnecessary worry? [interfaces/ecpg/ecpglib/prepare.c] static int nextStmtID = 1; static stmtCacheEntry *stmtCacheEntries = NULL; static struct declared_statement *g_declared_list; Regards Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Hi Meskes-san cc: Takahashi-san, Kuroda-san, Ideriha-san I attach a new patch. Please review it. Excuse: It doesn't include regression tests and pass them. Because I must reset all expected C program of regression. # I add an argument to ECPGdo(). I explain the patch as follows: 1. Specification It accepts the following .pgc. I confirmed it works well for AT clause. All results for st1 and st2 are same. exec sql prepare st0 as select 1; exec sql prepare st1(int,int) as select $1 + 5 + $2; exec sql prepare st2 from "select ? + 5 + ?"; exec sql prepare st3(bytea) as select octet_length($1); exec sql execute st0 into :ovar; exec sql execute st1(:var1,:var2) into :ovar; exec sql execute st1(11, :var2) into :ovar; exec sql execute st2(:var1,:var2) into :ovar; exec sql execute st2(11, :var2) into :ovar; exec sql execute st1 into :ovar using :var1,:var2; exec sql execute st2 into :ovar using :var1,:var2; exec sql execute st3(:b) into :ovar; 2. Behavior of ecpglib (1) PREPARE with AS clause Ecpglib sends the PREPARE statement to backend as is. (using PQexec). (2) EXECUTE with parameter list Ecpglib sends the EXECUTE statement as is (using PQexec), but all host variables in the list are converted to string-formatted and embedded into the EXECUTE statement. (3) PREPARE with FROM clause (not changed) Ecpglib sends 'P' libpq-message with statement (using PQprepare). (4) EXECUTE without parameter list (not changed) Ecpglib sends 'B' libpq-message with parameters. (using PQexecPrepared). 3. Change of preprocessor - I add ECPGst_prepare and ECPGst_execnormal. ECPGst_prepare is only for (1) and ECPGst_execnormal is only for (2). # I think the names are not good. - I add one argument to ECPGdo(). It's for prepared statement name. 4. I wonder whether I should merge (3) to (1) and (4) to (4) or not. Regards Ryo Matsumura ecpg_prepare_as_v1_1.patch Description: ecpg_prepare_as_v1_1.patch
RE: ECPG regression with DECLARE STATEMENT support
Hi Kurokawa-san I reviewd it. It's ok. I also confirm there is no same bug. Regards Ryo Matsumura
RE: ECPG regression with DECLARE STATEMENT support
Hi Kuroda-san I think that the 2nd argument of following ecpg_init() must be real_connection_name. Is it right? ECPGdeallocate(int lineno, int c, const char *connection_name, const char *name) : con = ecpg_get_connection(real_connection_name); if (!ecpg_init(con, connection_name, lineno)) ^^^ Regards Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Hi Meskes-san Thank you for your comment. I write three points in this mail. 1. > This also seems to be conflicting with > bd7c95f0c1a38becffceb3ea7234d57167f6d4bf. If we want to keep this > commit in for the release, I think we need to get these things fixed. I understand it. My idea is that add an argument for statement-name to ECPGdo() or add a new function that is a wrapper of ECPGdo() and has the argument. The argument is used for lookup related connection. Is it good? 2. > I wrote: > But it doesn't allow to use host variable in parameter clause of EXECUTE > statement like the following. I found a way to support host variables in parameter list of EXECUTE statement. ecpg_build_params() replace each parameter to string-formatted data that can be created by ecpg_store_input(). I will try it. 3. I found a bug in my patch. Replacing $ to @ in AS clause is not good because @ is absolute value operator. Therefore, the replacing cannot accept valid statement like the following. exec sql prepare st(int) select $1 + @1; -- It equals to "select $1 + 1" I choose $$1 unavoidably. Other character seems to be used as any operator. P.S. - PREPARE with FROM is the standard for Embedded SQL. - PREPARE with AS is not defined in the standard. - PREPARE with AS clause is PostgreSQL style. - Oracle and MySQL support only the standard. Regards Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Hi Meskes-san
Thank you for your advice.
I attach a patch.
I didn't add additional tests to regression yet.
The patch allows the following:
exec sql prepare(int) as select $1;
exec sql execute st(1) into :out;
exec sql prepare(text, text) as select $1 || $2;
exec sql execute st('aaa', 'bbb') into :out;
But it doesn't allow to use host variable in parameter clause of EXECUTE
statement like the following.
I'm afraid that it's not usefull. I will research the standard and other RDBMS.
If you have some information, please adivise to me.
exec sql begin declare section;
int var;
exec sql end declare section;
exec sql prepare(int) as select $1;
exec sql execute st(:var) into :out;
SQL error: bind message supplies 1 parameters, but prepared statement ""
requires 0
I explain about the patch.
* PREPARE FROM or PREPARE AS without type clause
It uses PQprepare(). It's not changed.
[Preprocessor output]
/* exec sql prepare st from "select ?"; */
{ ECPGprepare(__LINE__, NULL, 0, "st", "select ?");
/* exec sql prepare st as select 1; */
{ ECPGprepare(__LINE__, NULL, 0, "st", " select 1 ");
* PREPARE AS with type clause
It doesn't use PQprepare() but uses PQexecuteParams().
[Preprocessor output]
/* exec sql prepare st(text, text) as select $1 || '@2'; */
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "prepare \"st\" ( text ,
text ) as select @1 || '@2'", ECPGt_EOIT, ECPGt_EORT);
$1 in as clause is replaced by preprocessor at ecpg_param rule.
@1 is replaced to $1 by ecpglib at end of ecpg_build_params().
* EXECUTE without type clause
It uses PQexecPrepared(). It's not changed.
[Preprocessor output]
/* exec sql execute st into :ovar using :var; */
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_execute, "st",
ECPGt_int,&(var),(long)1,.
* EXECUTE with parameter clause
It uses PQexecuteParams().
[Preprocessor output]
/* exec sql execute st('abcde') into :ovar_s; */
{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "execute \"st\" ( 'abcde'
)", ECPGt_EOIT,
.
This approach causes the above constraint that users cannot use host variables
in parameter clause in EXECUTE statement
because ecpglib sends 'P' message with "execute \"st\" ($1)" and sends 'B' one
parameter, but backend always regards the number of parameters in EXECUTE
statement as zero.
I don't have any other idea...
Regards
Ryo Matsumura
ecpg_prepare_as_v1_0.patch
Description: ecpg_prepare_as_v1_0.patch
RE: SQL statement PREPARE does not work in ECPG
Hi Meskes-san
I must use a midrule action like the following that works as expected.
I wonder how to write the replacement to ecpg.addons.
I think it's impossible, right? Please give me some advice.
PrepareStmt:
PREPARE prepared_name prep_type_clause AS
{
prepared_name = $2;
prepare_type_clause = $3;
is_in_preparable_stmt = true;
}
PreparableStmt
{
$$.name = prepared_name;
$$.type = prepare_type_clause;
$$.stmt = $6;
is_in_preparable_stmt = false;
}
| PREPARE prepared_name FROM execstring
Regards
Ryo Matsumura
> -Original Message-
> From: Michael Meskes [mailto:mes...@postgresql.org]
> Sent: Friday, March 1, 2019 8:42 PM
> To: Matsumura, Ryo/松村 量 ; Takahashi,
> Ryohei/高橋 良平 ;
> 'pgsql-hack...@postgresql.org'
> Subject: Re: SQL statement PREPARE does not work in ECPG
>
> Hi Matsumura-san,
>
> > I will read README.parser, ecpg.addons, and *.pl to understand.
>
> Feel free to ask, when anything comes up.
>
> Michael
> --
> Michael Meskes
> Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
> Meskes at (Debian|Postgresql) dot Org
> Jabber: michael at xmpp dot meskes dot org
> VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
>
RE: SQL statement PREPARE does not work in ECPG
Meskes-san Thank you for your comment. > The only way to add this is by creating a replacement production for > this rule. parse.pl cannot do it itself. I will read README.parser, ecpg.addons, and *.pl to understand. > > I will use @1 instend of $$1 because the replacing is almost same as > > the existing replacing function in ecpglib. > > Is it good? > > I'd say so. I try it. Regards Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Hi Meskes-san, Takahashi-san
> If the standard allows it, we want to be able to process it.
I will try to implement it with the Idea-2 that doesn't use PQprepare() and
Takahasi-san's following idea.
> For example,
> - ECPG convert ":ID" to "$1" and "$1" in the original statement to "$$1"
> - next_insert() do not check "$$1"
> - ECPGdo() reconvert "$$1" to "$1"
But I will probably be late because I don't understand parse.pl very well.
I think that the following rule is made by parse.pl.
PreparableStmt:
SelectStmt
{
is_in_preparable_stmt = true; <--- I want to add it.
$$ = $1;
}
| InsertStmt
.
The above variable is used in ecpg.trailer.
ecpg_param: PARAM {
if(is_in_preparable_stmt)
$$ = mm_strdup(replace_dollar_to_something());
else
$$ = make_name();
} ;
I will use @1 instend of $$1 because the replacing is almost same as the
existing replacing function in ecpglib.
Is it good?
Regards
Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Meskes-san
I made mistake.
> The checking (line-1495) is meaningless for AS clause.
> It checks if all $0 is replaced to literal and all ? is replaced to $[0-9]*
> by insert_tobeinserted(),
> but it always fails because $[0-9]* in AS clause are not replaced (and should
> not be replaced).
> I don't search if there is other similar case. It is Idea-2.
It checks if a number of variables equals a number of $* after replacing $0 and
?.
It always fails because there is no variable for $* in AS clause.
We should skip AS clause at the cheking.
Umm... The skipping seems to be not easy too.
next_insert(char *text, int pos, bool questionmarks, bool std_strings)
{
pos = get_pos_of_as_clause(text); <-- parse text in ecpglib???
for (; text[p] != '\0'; p++)
if(is_prepare_statement(stmt) && invalid_pos(pos))
break;
Regards
Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Meskes-san
> Did you analyze the bug? Do you know where it comes from?
At first, I show the flow of Prepare statement without AS clause and
the flow of Prepare statement with AS clause but without parameter list.
[preproc/preproc.y]
1832 | PrepareStmt
1834 if ($1.type == NULL || strlen($1.type) == 0)
1835 output_prepare_statement($1.name, $1.stmt);
[preproc/output.c]
168 output_prepare_statement(char *name, char *stmt)
169 {
170 fprintf(base_yyout, "{ ECPGprepare(__LINE__, %s, %d, ", connection ?
connection : "NULL", questionmarks);
171 output_escaped_str(name, true);
172 fputs(", ", base_yyout);
173 output_escaped_str(stmt, true);
174 fputs(");", base_yyout);
It makes the following C-program and it can work.
/* exec sql prepare st as select 1; */
ECPGprepare(__LINE__, NULL, 0, "st", " select 1 ");
/* exec sql prepare st from "select 1"; */
ECPGprepare(__LINE__, NULL, 0, "st", "select 1");
/* exec sql prepare st from "select ?"; */
ECPGprepare(__LINE__, NULL, 0, "st", "select ?");
ecpglib processes as the following:
[ecpglib/prepare.c]
174 ECPGprepare(int lineno, const char *connection_name, const bool
questionmarks,
175 const char *name, const char *variable)
199 this = ecpg_find_prepared_statement(name, con, );
200 if (this && !deallocate_one(lineno, ECPG_COMPAT_PGSQL, con, prev, this))
201 return false;
203 return prepare_common(lineno, con, name, variable);
[ecpglib/prepare.c]
115 prepare_common(int lineno, struct connection *con, const char *name, const
char *variable)
135 stmt->lineno = lineno;
136 stmt->connection = con;
137 stmt->command = ecpg_strdup(variable, lineno);
138 stmt->inlist = stmt->outlist = NULL;
141 replace_variables(&(stmt->command), lineno);
144 this->name = ecpg_strdup(name, lineno);
145 this->stmt = stmt;
148 query = PQprepare(stmt->connection->connection, name, stmt->command, 0,
NULL);
The following is log of PQtrace().
To backend> Msg P
To backend> "st"
To backend> "select $1"
To backend (2#)> 0
[6215]: prepare_common on line 21: name st; query: "select $1"
An important point of the route is that it calls PQprepare() and PQprepare()
needs type-Oid list. (Idea-1) If we fix for Prepare statement with AS clause and
with parameter list to walk through the route, preprocessor must parse the
parameter list and
preprocessor or ecpglib must make type-Oid list. I think it's difficult.
Especially, I wonder if it can treat user defined type and complex structure
type.
At second, I show the flow of Prepare statement with AS clause.
1836 else
1837 output_statement(cat_str(5, mm_strdup("prepare"), $1.name, $1.type,
mm_strdup("as"), $1.stmt), 0, ECPGst_normal);
It makes the following C-program, but it cannot work because AS clause is
double quoted.
So there is no work-around for this route.
/* exec sql prepare st(int) as select $1; */
ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "prepare \"st\" ( int ) as \"
select $1 \"", ECPGt_EOIT, ECPGt_EORT);
When it runs, the following error is occured.
[5895]: raising sqlcode -202 on line 20: too few arguments on line 20
SQL error: too few arguments on line 20
The following may be expected.
ECPGdo(__LINE__, 0 , 1, NULL, 0, ECPGst_normal, "prepare st ( int ) as select
$1 ", ECPGt_EOIT, ECPGt_EORT);
Even if the above C-program is made, another error is occured.
The error is occured in the following flow.
[ecpglib/execute.c]
1196 ecpg_build_params(struct statement *stmt)
1214 var = stmt->inlist;
1215 while (var)
ecpg_store_input(var--->tobeinserted)
1393 if ((position = next_insert(stmt->command, position,
stmt->questionmarks, std_strings) + 1) == 0)
1411 if (var->type == ECPGt_char_variable)
1413 int ph_len = (stmt->command[position] == '?') ?
strlen("?") : strlen("$1");
1415 if (!insert_tobeinserted(position, ph_len, stmt, tobeinserted))
1428 else if (stmt->command[position] == '0')
1430 if (!insert_tobeinserted(position, 2, stmt, tobeinserted))
1437 else
1468 if (stmt->command[position] == '?')
1480 snprintf(tobeinserted, buffersize, "$%d", counter++);
1474 if (!(tobeinserted = (char *) ecpg_alloc(buffersize,
stmt->lineno)))
1492 var = var->next;
1493 }
1495 /* Check if there are unmatched things left. */
1496 if (next_insert(stmt->command, position, stmt->questionmarks,
std_strings) >= 0)
1497 {
1498 ecpg_raise(stmt->lineno, ECPG_TOO_FEW_ARGUMENTS,
1499ECPG_SQLSTATE_USING_CLAUSE_DOES_NOT_MATCH_PARAMETERS,
NULL);
*** The above is raised. ***
The checking (line-1495) is meaningless for AS clause.
It checks if all $0 is replaced to literal and all ? is replaced to $[0-9]* by
insert_tobeinserted(),
but it always fails because $[0-9]* in AS clause are not replaced (and
RE: SQL statement PREPARE does not work in ECPG
Hi, Maybe, there is no work-around. For supporting it, there are two steps. step1. fix for PREPARE. step2. fix for EXECUTE. About step1, there are two way. I want to choose Idea-2. Idea-1. ecpglib prepares Oids of type listed in PREPARE statement for 5th argument of PQprepare(). But it's difficult because ecpg has to know Oids of type. # Just an idea, create an Oid list in parsing. Idea-2. Use ECPGdo with whole PREPARE statement. In this way, there is no problem about Oid type because backend resolves it. I think the current implementation may aim to it. If we choose Idea-2, we should make a valid SQL-command(remove double quotation) and avoid any processing about prep_type_clause and PreparableStmt except for parsing. One of such processing is the checking a number of parameters that occured the error. It may take time, but it's easier than Idea-1. Is the direction of fixing good? About step2, there is the work-arround pointed by Meskes-san. Regards Ryo Matsumura
RE: SQL statement PREPARE does not work in ECPG
Hi
I think SQL statement PREPARE *without* parameter is supported,
but one with parameter is not supported (or has some fatal bugs).
Because route for SQL statement PREPARE (line-1837 of preproc.y) always has
output an invalid SQL statement and
there is no regression test for SQL statement PREPARE.
[preproc.y]
1832 | PrepareStmt
1833 {
1834 if ($1.type == NULL || strlen($1.type) == 0)
1835 output_prepare_statement($1.name, $1.stmt);
1836 else
1837 output_statement(cat_str(5, mm_strdup("prepare"), $1.name,
$1.type, mm_strdup("as"), $1.stmt), 0, ECPGst_normal);
1838 }
The next is log of ECPGdebug() and PQtrace() for the following statement.
exec sql prepare st(int) as select col1 from foo;
[14968]: ecpg_execute on line 17: query: prepare "st" ( int ) as " select 1 ";
with 0 parameter(s) on connection conn
To backend> Msg Q
To backend> "prepare "st" ( int ) as " select 1 ""
To backend> Msg complete, length 42
2019-02-19 06:23:30.429 UTC [14969] ERROR: syntax error at or near "" select 1
"" at character 25
2019-02-19 06:23:30.429 UTC [14969] STATEMENT: prepare "st" ( int ) as "
select 1 "
Regards
Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san > This looks very good. Committed to HEAD. Thank you so match. Regards Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san I attach a new patch. - ECPGset_desc and ecpg_build_params are changed. - implement Idea-2. - get if(is_binary)-block out from main flow to new functions (set_desc_attr, store_input_from_desc). Regards Ryo Matsumura ecpg_bytea_v1_4.patch Description: ecpg_bytea_v1_4.patch
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san
> Yes, I agree with this. But it does not explain why we cannot just add
> a length parameter. And it neither explains why we need so many if
> (!bytea) { thisandthat } else { somethingelse } blocks. I would prefer
> the integration to be smoother. Hopefully that is possible.
I agree that the special route is ugly, but I cannot remove them completely.
I try to implement Idea-2. In same time, I try to move if(bytea) blocks to
new function for readability.
e.g. move the following to new function set_data_attr().
if (var->type != ECPGt_bytea)
desc_item->is_binary = false;
else
{
struct ECPGgeneric_varchar *variable =
(struct ECPGgeneric_varchar *) (var->value);
desc_item->is_binary = true;
desc_item->data_len = variable->len;
}
ecpg_free(desc_item->data);
desc_item->data = (char *) tobeinserted;
Regards
Ryo Matsumura
> -Original Message-
> From: Michael Meskes [mailto:mes...@postgresql.org]
> Sent: Wednesday, February 13, 2019 9:09 PM
> To: Matsumura, Ryo/松村 量
> Cc: Tsunakawa, Takayuki/綱川 貴之 ;
> pgsql-hackers@lists.postgresql.org
> Subject: Re: [PROPOSAL]a new data type 'bytea' for ECPG
>
> Matsumura-san,
>
> > Current architecture:
> > Internal expression of varchar is C-string that includes length
> > information implicitly
> > because the length can be computed by strlen().
> > ECPGdo(ecpg_build_params) assumes that the data in descriptor is C-
> > string encoded.
> >
> > In other hand, bytea data is binary that doesn't include any length
> > information.
> > And the merit of my proposal is that bytea data can be sent to
> > backend without
> > C-string encodeing overhead. They are different points from varchar.
>
> Yes, I agree with this. But it does not explain why we cannot just add
> a length parameter. And it neither explains why we need so many if
> (!bytea) { thisandthat } else { somethingelse } blocks. I would prefer
> the integration to be smoother. Hopefully that is possible.
>
> > My Idea-2 is that:
> > - ECPGset_desc copies data to descriptor_item.data, set the length to
> > dscriptor_item.data_len and set type information to
> > descriptor_item.is_binary.
> > - ecpg_build_params only memcpy as folowing without ecpg_store_input:
> >
> > if (descriptor_item.is_binary)
> > memcpy(, descriptor_item.data,
> > descriptor_item.data_len)
>
> Isn't that a better way then? This looks more smoothly to me.
>
> Michael
> --
> Michael Meskes
> Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
> Meskes at (Debian|Postgresql) dot Org
> Jabber: michael at xmpp dot meskes dot org
> VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
>
>
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san
At first, I find my mistake that the following member is not used in my patch.
Sorry...
[ecpglib_extern.h]
120 struct descriptor_item
130 int data_len;
> Why is handling a bytea so different from handling a varchar?
Current architecture:
Internal expression of varchar is C-string that includes length information
implicitly
because the length can be computed by strlen().
ECPGdo(ecpg_build_params) assumes that the data in descriptor is C-string
encoded.
In other hand, bytea data is binary that doesn't include any length information.
And the merit of my proposal is that bytea data can be sent to backend without
C-string encodeing overhead. They are different points from varchar.
I try to explain current data flow and my ideas.
# It may not be simple explanation...
Current data flow:
> /* exec sql set descriptor idesc value 1 data = :binary_var; */
> { ECPGset_desc(__LINE__, "idesc", 1,ECPGd_data,
> ECPGt_bytea,&(binary_var),(long)DATA_SIZE,(long)1,sizeof(struct bytea_1),
> ECPGd_EODT);
Ecpglib stores user data into [struct descriptor_item].
Ecpglib stores only C-string encoded data to 'data' member with
ecpg_store_input.
Of course, if user specifies length(*), ecpg_store_input strncpy() with the
length.
(*)len member of struct varchar_1 { int len; char arr[ DATA_SIZE ]; }
# desctiptor_item has 'type' and 'length' member. But the above statement
doesn't set
# these fields because I think they should be set by user explicitly as the
following:
# exec sql set descriptor idesc value 1 length = 3;
# I explain later that the above user statement is ignored in result.
> /* exec sql execute ins_stmt using sql descriptor idesc; */
> { ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_execute, "ins_stmt",
> ECPGt_descriptor, "idesc", 1L, 1L, 1L,
> ECPGt_NO_INDICATOR, NULL , 0L, 0L, 0L, ECPGt_EOIT, ECPGt_EORT);
ecpg_build_params, the first step of ECPGdo, only strcpy() from
descriptor_item.data to
tobeinserted by ecpg_store_input because the input [struct variable] for
ecpg_store_input
is always set type='ECPGt_char'. descriptor_item.type and
descriptor_item.length are
always not used.
# varcharsize member is set to value of strlen(descriptor_item.data) but it's
ignored
# by ecpg_store_input.
In that flow, how user binary data is set to tobeinserted without C-string
encoding?
The premise are the followings:
- The length information set by user must be inform upto ecpg_build_params.
- The media of the length information from ECPGset_desc to ECPGdo is only
[struct descriptor_item].
My Idea-1 in the previous mail is that:
- ECPGset_desc copies whole of the struct(*) to descriptor_item.data and sets
type
information to descriptor_item.is_binary.
(*)bytea_a { int len; char arr[DATA_SIZE]; }
- ecpg_build_params calls ecpg_store_input for the descriptor_item.data just as
the folowing input variable.
execute sql insert into foo values(:binary_var);
My Idea-2 is that:
- ECPGset_desc copies data to descriptor_item.data, set the length to
dscriptor_item.data_len and set type information to descriptor_item.is_binary.
- ecpg_build_params only memcpy as folowing without ecpg_store_input:
if (descriptor_item.is_binary)
memcpy(, descriptor_item.data, descriptor_item.data_len)
Thank you.
Ryo Matsumura
> -Original Message-
> From: Michael Meskes [mailto:mes...@postgresql.org]
> Sent: Tuesday, February 12, 2019 11:06 PM
> To: Matsumura, Ryo/松村 量
> Cc: Tsunakawa, Takayuki/綱川 貴之 ;
> pgsql-hackers@lists.postgresql.org
> Subject: Re: [PROPOSAL]a new data type 'bytea' for ECPG
>
> Matsumura-san,
>
> > I try to explain as follows. I would like to receive your comment.
> > ...
>
> I'm afraid I don't really understand your explanation. Why is handling
> a bytea so different from handling a varchar? I can see some
> differences due to its binary nature, but I do not understand why it
> needs so much special handling for stuff like its length? There is a
> length field in the structure but instead of using it the data field is
> used to store both, the length and the data. What am I missing?
>
> Please keep in mind that I did not write the descriptor code, so I may
> very well not see the obvious.
>
> Michael
> --
> Michael Meskes
> Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
> Meskes at (Debian|Postgresql) dot Org
> Jabber: michael at xmpp dot meskes dot org
> VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
>
>
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san Thank you for your review. > There is one thing that I don't understand right now. YOu > change ecpg_store_input() to handle the bytea data type, yet you also > change ECPGset_desc() to not use ecpg_store_input() in case of an > bytea. This looks odd to me. Can you please explain that to me? I try to explain as follows. I would like to receive your comment. The current architecture of data copying of descriptor walks through the following path. The important point is that it walks through two ecpg_store_input(). step 1. ECPGset_desc Store to descriptor_item with ecpg_store_input(). step 2. ecpg_build_params(setup for tobeinserted) Store to tobeinserted with ecpg_store_input(). step 3. ecpg_build_params(building stmt->param*) Set tobeinserted to stmt->paramvalues. On the other hand, the part of ecpg_build_params(building stmt->param*) for bytea needs two information that are is_binary and binary_length. But, data copying with ecpg_store_input() losts them. There are two ideas to pass the information to part of ecpg_build_params(building stmt->param*). But they are same in terms of copying data without ecpg_store_input() at least ones. I selected Idea-1. Idea-1. step 1. ECPGset_desc Set descriptor_item.is_binary. Memcpy both bytea.length and bytea.array to descriptor_item.data. step 2. ecpg_build_params(setup for tobeinserted) Store bytea.array to tobeinserted with ecpg_store_input(bytea route). Set is_binary(local) from descriptor_item.is_binary. Set binary_length(local) from descriptor_item.data. step 3. ecpg_build_params(building stmt->param*) Set stmt->paramvalues from tobeinserted. Set stmt->formats from is_binary(local). Set stmt->lengths from binary_length(local). Idea-2. step 1. ECPGset_desc Set descriptor_item.is_binary. Set bytea.length to descriptor_item.data_len. (different!) Set bytea.array to descriptor_item.data. (different!) step 2. ecpg_build_params(setup for tobeinserted) Memcpy bytea.array to tobeinserted by using alloc and memcpy whitout store_input. (different!) Set is_binary(local) from descriptor_item.is_binary. Set binary_length(local) from descriptor_item.data_len. (different!) step 3. ecpg_build_params(building stmt->param*) Set stmt->paramvalues with tobeinserted. Set stmt->formats from is_binary(local). Set stmt->lengths from binary_length(local). Regards Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san Thank you for your comment. I remove it and attach a new patch. Please review it. I feel sorry for asking you to reveiw without contribution. Regards Ryo Matsumura > -Original Message- > From: Michael Meskes [mailto:mes...@postgresql.org] > Sent: Friday, February 1, 2019 6:29 PM > To: Matsumura, Ryo/松村 量 > Cc: 'Michael Meskes' ; Tsunakawa, Takayuki/綱川 貴 > 之 ; pgsql-hackers@lists.postgresql.org > Subject: Re: [PROPOSAL]a new data type 'bytea' for ECPG > > Matsumura-san, > > > Sorry to bother you, but I would be grateful if you would comment to me. > > Sorry, I didn't know you were waiting on a reply by me. > > > > I have a question about ecpg manual when I add article for bytea. > > > I wonder what does the following about VARCHAR mean. > > > ... > > > I think, if the footnote of VARCHAR is meaningless, I remove it while I > add > > > the article for bytea. (I didn't remove in this patch.) > > I have no idea where the footnote comes from, but I agree that it doesn't seem > to make sense. The datatype varchar in the C code is handled by the > preprocessor and replaced by a struct definition anyway. > > Feel free to remove. > > Michael > -- > Michael Meskes > Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) > Meskes at (Debian|Postgresql) dot Org > Jabber: michael at xmpp dot meskes dot org > VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL > ecpg_bytea_v1_3.patch Description: ecpg_bytea_v1_3.patch
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san Sorry to bother you, but I would be grateful if you would comment to me. Regards Ryo Matsumura > -Original Message- > From: Matsumura, Ryo [mailto:matsumura@jp.fujitsu.com] > Sent: Wednesday, December 19, 2018 7:05 PM > > Cc: pgsql-hackers@lists.postgresql.org > Subject: RE: [PROPOSAL]a new data type 'bytea' for ECPG > > Meskes-san > > > > I do think, though, we should change the debug output for > > > ecpg_free_params(). > > > > I try to change about it. Next patch will print binary in hex-format. > > I implement and attach it. Please review a new patch in this mail. > > > > I have a question about ecpg manual when I add article for bytea. > I wonder what does the following about VARCHAR mean. > > 35.4.4. Type Mapping > Table 35.1. Mapping Between PostgreSQL Data Types and C Variable Types > > character(n), varchar(n), text | char[n+1], VARCHAR[n+1] [b] > > [b] declared in ecpglib.h > > There is no declaration for VARCHAR in ecpglib.h. > There is a declaration for ECPGt_varchar in ecpgtype.h, but it may be > be unusefull information for users. > > On the other hand, footnote for 'bool' is usefull because there is > C-definition macro in ecpglib.h. > > > [c] declared in ecpglib.h if not native > > > #ifndef bool > #define bool char > #endif > > I think, if the footnote of VARCHAR is meaningless, I remove it while I add > the article for bytea. (I didn't remove in this patch.) > > Regards > Ryo Matsumura
RE: [suggestion]support UNICODE host variables in ECPG
> * What's the benefit of supporting UTF16 in host variables? I think that the first benefit of suggestion is providing a way to treat UTF16 chars for application. Whether or not to support above U+ (e.g. surrogate pair) may be a next discussion. For that purpose, implementation for the suggestion may be easier than for supporting UTF16 at client_encoding. Uvarchar seems to be a label indicating that stored data is encoded by UTF16. It localizes the impacts within only labeled host variable. # At least, ecpglib is not good at treating 0x00 as a part of one character. Regards Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san > > I do think, though, we should change the debug output for > > ecpg_free_params(). > > I try to change about it. Next patch will print binary in hex-format. I implement and attach it. Please review a new patch in this mail. I have a question about ecpg manual when I add article for bytea. I wonder what does the following about VARCHAR mean. 35.4.4. Type Mapping Table 35.1. Mapping Between PostgreSQL Data Types and C Variable Types character(n), varchar(n), text | char[n+1], VARCHAR[n+1] [b] [b] declared in ecpglib.h There is no declaration for VARCHAR in ecpglib.h. There is a declaration for ECPGt_varchar in ecpgtype.h, but it may be be unusefull information for users. On the other hand, footnote for 'bool' is usefull because there is C-definition macro in ecpglib.h. [c] declared in ecpglib.h if not native #ifndef bool #define bool char #endif I think, if the footnote of VARCHAR is meaningless, I remove it while I add the article for bytea. (I didn't remove in this patch.) Regards Ryo Matsumura ecpg_bytea_pg_v1_2.patch Description: ecpg_bytea_pg_v1_2.patch ecpg_bytea_test_v1_2.patch Description: ecpg_bytea_test_v1_2.patch ecpg_bytea_doc_v1_2.patch Description: ecpg_bytea_doc_v1_2.patch
RE: [suggestion]support UNICODE host variables in ECPG
Nagaura-san I understand that the previous discussion pointed that the feature had better be implemented more simply or step-by-step and description about implementation is needed more. I also think it prevented the discussion to reach to the detail of feature. What is your opinion about it? Regards Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san I noticed that I was confused. My topic was about adding bytea as new host variable type. The topic *didn't* include that receiving binary format data into SQLDATA descriptor like the following. sqlda_t *sqlda; exec sql create table if not exists test (c1 bytea); exec sql select c1 into descriptor sqlda from test; /* It expects that sqlda->sqlvar[0].sqldata is binary format. */ So, please ignore the following in my mail at 2018-11-12 02:14:58. > P.S. > The patch does not support ECPG.bytea in sqltype of "struct sqlvar_struct" > because of compatibility. The topic included that receiving binary data into Named SQL descriptor with using bytea host variable like the following. It has already been implemented in my patch. exec sql begin declare section; bytea var[128]; exec sql end declare section; exec sql create table if not exists test (c1 bytea); exec sql allocate descriptor odesc; exec sql select c1 into sql descriptor odesc from test; exec sql get descriptor odesc value 1 :var = data; Regards Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san Maybe I understand your comments about compatibility. I will try to implement for sqlda. # I am not goot at library version mechanism. # I will learn about it. Regards Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san
> > > > The patch does not support ECPG.bytea in sqltype of "struct
> > > > sqlvar_struct"
> > > > because of compatibility.
> >
> > Sorry I do not really understand what you mean. Could you please
> > explain?
>
> I meaned that existing applications that receive data of bytea column
> with using sqlda will encounter their unknown type(=ECPG.bytea) in
> sqlvar_struct.sqltype.
>
> You mean if they are not recompiled? If so, yes, how else could that be
> handled?
Even if they are recompiled, they will fail.
switch (sqlvar_struct.sqltype)
{
case ECPG.int: break;
case ECPG.char: break;
/* There is no case for ECPG.bytea */
default: abort();
There is an idea as following, but it seems to be ugly.
Implement a parameter for ecpglib.
The parameter means whether application want to receive
bytea data in binary format or not. Default is "not".
# I don't know any ecpglib's parameter like it.
In other words, if application uses "bytea" type host variable,
ecpglib could know its intent, but in case of sqlda ecpglib could
not know it.
Regards
Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san Tnak you for your comment. > I do think, though, we should change the debug output for > ecpg_free_params(). I try to change about it. Next patch will print binary in hex-format. > > > The patch does not support ECPG.bytea in sqltype of "struct > > > sqlvar_struct" > > > because of compatibility. > > Sorry I do not really understand what you mean. Could you please > explain? I meaned that existing applications that receive data of bytea column with using sqlda will encounter their unknown type(=ECPG.bytea) in sqlvar_struct.sqltype. Regards Ryo Matsumura > -Original Message- > From: Michael Meskes [mailto:mes...@postgresql.org] > Sent: Wednesday, December 5, 2018 8:24 PM > To: Matsumura, Ryo/松村 量 ; Tsunakawa, > Takayuki/綱川 貴之 > Cc: pgsql-hackers@lists.postgresql.org > Subject: Re: [PROPOSAL]a new data type 'bytea' for ECPG > > Matsumura-san, > > > Sorry to bother you, but I hope any comment of yours. > > It is no bother. > > I'm fine with the patch if it does not work against the standard. > > I do think, though, we should change the debug output for > ecpg_free_params(). The way it is now it prints binary values which we > also have in our test suite. I'm afraid this will come back to haunt > us. > > > > The patch does not support ECPG.bytea in sqltype of "struct > > > sqlvar_struct" > > > because of compatibility. > > Sorry I do not really understand what you mean. Could you please > explain? > > Michael > -- > Michael Meskes > Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) > Meskes at (Debian|Postgresql) dot Org > Jabber: michael at xmpp dot meskes dot org > VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL > >
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Meskes-san Sorry to bother you, but I hope any comment of yours. Regards Ryo Matsumura > Subject: RE: [PROPOSAL]a new data type 'bytea' for ECPG > > > From: Tsunakawa, Takayuki [mailto:tsunakawa.ta...@jp.fujitsu.com] > > > > I think the host variable data type that corresponds to the server-side > > bytea > > should be bytea. As the following pages state or imply, it would be better > > to create standard-compliant LOB types someday, and use the keyword BLOB > in > > ECPG for that type. The server-side data types should have the names BLOB, > > CLOB and NCLOB. Those types should handle data larget than 1 GB and have > the > > locator feature defined in the SQL standard. Maybe we should also advanced > > LOB features like Oracle's SecureFiles LOB and SQL Server's FileTables. > > Tsunakawa-san, thanks for your advice. > I understand that C type definition of client-side bytea is not constrained > by the standard BLOB. > > What should I do next? > For now, I attach a patch that is removed noise(pgindent/typedef.list). > > P.S. > The patch does not support ECPG.bytea in sqltype of "struct sqlvar_struct" > because of compatibility. > > Regards > Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
> From: Tsunakawa, Takayuki [mailto:tsunakawa.ta...@jp.fujitsu.com] > > I think the host variable data type that corresponds to the server-side bytea > should be bytea. As the following pages state or imply, it would be better > to create standard-compliant LOB types someday, and use the keyword BLOB in > ECPG for that type. The server-side data types should have the names BLOB, > CLOB and NCLOB. Those types should handle data larget than 1 GB and have the > locator feature defined in the SQL standard. Maybe we should also advanced > LOB features like Oracle's SecureFiles LOB and SQL Server's FileTables. Tsunakawa-san, thanks for your advice. I understand that C type definition of client-side bytea is not constrained by the standard BLOB. What should I do next? For now, I attach a patch that is removed noise(pgindent/typedef.list). P.S. The patch does not support ECPG.bytea in sqltype of "struct sqlvar_struct" because of compatibility. Regards Ryo Matsumura ecpg_bytea_v1_1.patch Description: ecpg_bytea_v1_1.patch
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Hi Michael > > In Pro*C, the data should be represented as hex format C string. > > Just to clarify, there is no special datatype for binary data? I apology for lack of research again. Since it's a little difficult to answer, I explain by samples. The following works. unsigned char buffer[128]; /* It's not needed to declare in DECLARE section. */ exec sql var buffer is raw(128); /* This sematics may be different in ECPG. */ exec sql create table test(c1 raw(128)); exec sql insert into test(c1) values(:buffer); The following cannot be pre-compiled. In this sence, there is no special datatype for binary data. exec sql begin declare section; raw buffer[128]; /* error */ exec sql end declare section; exec sql create table test(c1 raw(128)); exec sql insert into test(c1) values(:buffer); > > bytea as a type of table definition may correspond to BLOB in the > > standard. > > Would we prefer to add a blob datatype then? I think that blob datatype is needed for large binary data *in finally*, but blob datatype and its access methods(*) is not needed for non-large binary data(e.g. use for text data of which encoding is different from database encoding) because of its complexity. My proposal is mainly for non-large binary data. (*) e.g. In Pro*C, OPEN, READ, WRITE, CLOSE, APPEND, and so on. > > It seems that there is no defact and no product following to the > > standards. > > I wonder whether bytea should follow to the standard completely or > > follow to existing varchar for usability. > > Do you see any disadvantage of following the standard? I don't really > see where the usability drawback is. In general I would prefer being as > close to the standard as reasonably possible. I think there is no special reason to follow to existing varchar. I can accept the standard. (Re-implementation is not difficult.) Thank you. Regards Ryo Matsumura > -Original Message- > From: Michael Meskes [mailto:mes...@postgresql.org] > Sent: Saturday, October 27, 2018 3:43 AM > To: Matsumura, Ryo/松村 量 ; > pgsql-hackers@lists.postgresql.org > Subject: Re: [PROPOSAL]a new data type 'bytea' for ECPG > > Hi Ryo-san, > > > # Please call me Ryo. Matsumura is too long. > > Thanks. > > > In Pro*C, the data should be represented as hex format C string. > > Just to clarify, there is no special datatype for binary data? > > > bytea as a type of table definition may correspond to BLOB in the > > standard. > > Would we prefer to add a blob datatype then? > > > It seems that there is no defact and no product following to the > > standards. > > I wonder whether bytea should follow to the standard completely or > > follow to existing varchar for usability. > > Do you see any disadvantage of following the standard? I don't really > see where the usability drawback is. In general I would prefer being as > close to the standard as reasonably possible. > > Michael > -- > Michael Meskes > Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) > Meskes at (Debian|Postgresql) dot Org > Jabber: michael at xmpp dot meskes dot org > VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL >
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Hi Michael
Thank you for your comment.
# Please call me Ryo. Matsumura is too long.
> > I think that set/put data for host variable should be more simple.
> > The following is an example of Oracle Pro *C program for RAW type
> > column.
>
> Just to be clear, Oracle can use varchars for binary data, right?
I'm sorry. That is my mistake.
In Pro*C, the data should be represented as hex format C string.
> > In ECPG, varchar host variable cannot be used for bytea because it
> > cannot treat
> > '\0' as part of data. If the length is set to 10 and there is '\0' at
> > 3rd byte,
> > ecpglib truncates 3rd byte and later at the following:
>
> I've been traveling too much to check, but does the standard say
> anything about that?
bytea as a type of table definition may correspond to BLOB in the standard.
If it is true, the standard defines corresponding type in C as the following:
--
struct {
long hvn_reserved
unsigned long hvn_length
char hvn_data[L];
} hvn
* hvn is the name of the host variable defined to correspond
to the SQL data type
--
> > I also think that the behavior of varchar host variable should not be
> > changed
> > because of compatibility.
> > Therefore, a new type of host variable 'bytea' is needed.
> This I am actually not sure about. I think we should try to stick with
> the standard and, if it does not comment on it, with what others in the
> market do to make migrations easier. So far I do not remember any
> database having a bytea datatype in embedded SQL.
Maybe Oracle doesn't have it.
I found documents of DB2.
blob(n) correspond to BLOB in V11.
https://www.ibm.com/support/knowledgecenter/en/SSEPGG_11.1.0/com.ibm.db2.luw.apdv.embed.doc/doc/r0006090.html
Since I don't have DB2 installation, I cannot confirm typedef of blob(n).
But in V9 the following structure correspond to BLOB.
https://www.ibm.com/support/knowledgecenter/en/SSEPGG_9.7.0/com.ibm.db2.luw.apdv.routines.doc/doc/c0009190.html
struct sqludf_lob
{
sqluint32 length; /* length in bytes */
char data[1]; /* first byte of lob */
};
It seems that there is no defact and no product following to the standards.
I wonder whether bytea should follow to the standard completely or
follow to existing varchar for usability.
Thank you.
Regards
Ryo Matsumura
RE: [PROPOSAL]a new data type 'bytea' for ECPG
Hackers
No one commented to the proposal, but I'm not discouraged.
I attach a patch. Please review or comment to proposal.
Note:
- The patch cannot yet decode escape format data from backend.
- [ecpg/test/expected/sql-bytea.stderr] in patch includes non-ascii data.
I explain a little about the patch.
Preproc:
Almost same as varchar.
Ecpglib:
- ecpg_build_params()
Build two more arrays paramlengths and paramformats for PQexecParams().
If input variable type is bytea, set pramformats to 1(= is binary) and
set binary data length to paramlengths.
- ecpg_store_input()
If input variable type is bytea, copy its binary data to ecpg_alloc-ed area
directly.
- ecpg_get_data()
If output variable type is bytea, decode received results to user area.
Encode/decode function is imported from backend/utils/adt/encode.c
- ECPGset_desc()
Currently ecpglib saves data to internal area(struct descriptor_item) for
execution phase,
but doesn't save type information that is needed in case of bytea.
So I add a member is_binary to descriptor_item structure.
Thank you.
Regards
Ryo Matsumura
> -Original Message-
> From: Matsumura, Ryo [mailto:matsumura@jp.fujitsu.com]
> Sent: Monday, October 1, 2018 5:04 PM
> To: pgsql-hackers@lists.postgresql.org
> Subject: [PROPOSAL]a new data type 'bytea' for ECPG
>
> Hi, Hackers
>
> # This is my first post.
>
> I will try to implement a new data type 'bytea' for ECPG.
> I think that the implementation is not complicated.
> Does anyone need it ?
>
>
> * Why do I need bytea ?
>
> Currently, ECPG program can treat binary data for bytea column with 'char'
> type
> of C language, but it must convert from/to escaped format with
> PQunescapeBytea/
> PQescapeBytea(). It forces users to add an unnecessary code and to pay cost
> for
> the conversion in runtime.
> # My PoC will not be able to solve output conversion cost.
>
> I think that set/put data for host variable should be more simple.
> The following is an example of Oracle Pro *C program for RAW type column.
>
> VARCHAR raw_data[20];
>
> /* preprocessed to the following
>* struct
>* {
>*unsigned short len;
>*unsigned char arr[20];
>* } raw_data;
>*/
>
> raw_data.len = 10;
> memcpy(raw_data.arr, data, 10);
>
> see also:
>
> https://docs.oracle.com/cd/E11882_01/appdev.112/e10825/pc_04dat.htm#i2330
> 5
>
> In ECPG, varchar host variable cannot be used for bytea because it cannot
> treat
> '\0' as part of data. If the length is set to 10 and there is '\0' at 3rd
> byte,
> ecpglib truncates 3rd byte and later at the following:
>
> [src/interfaces/ecpg/ecpglib/execute.c]
> ecpg_store_input(const int lineno, const bool force_indicator, const struct
> :
> switch (var->type)
> :
> case ECPGt_varchar:
> if (!(newcopy = (char *) ecpg_alloc(variable->len + 1, lineno)))
> return false;
> !! strncpy(newcopy, variable->arr, variable->len);
> newcopy[variable->len] = '\0';
>
> I also think that the behavior of varchar host variable should not be changed
> because of compatibility.
> Therefore, a new type of host variable 'bytea' is needed.
>
> Since ecpglib can distinguish between C string and binary data, it can send
> binary data to backend directly by using 'paramFormats' argument of
> PQexecParams().
> Unfortunately, the conversion of output data cannot be omitted in ecpglib
> because
> libpq doesn't provide like 'paramFormats'.
> ('resultFormat' means that *all* data from backend is formatted by binary
> or not.)
>
> PQexecParams(PGconn *conn,
> const char *command,
> int nParams,
> const Oid *paramTypes,
> const char *const *paramValues,
> const int *paramLengths,
> !! const int *paramFormats,
> int resultFormat)
>
>
>
> * How to use new 'bytea' ?
>
> ECPG programmers can use almost same as 'varchar' but cannot use as names.
> (e.g. connection name, prepared statement name, cursor name and so on)
>
> - Can use in Declare Section.
>
> exec sql begin declare section;
> bytea data1[512];
> bytea data2[DATA_SIZE]; /* can use macro */
> bytea send_data[DATA_NUM][DATA_SIZE]; /* can use two dimensional array
> */
> bytea recv_data[][DATA_SIZE]; /* can use flexible array */
> exec sql end declare section;
>
> - Can *not* use for name.
>
> exec sql begin declare section;
> bytea conn_name[DATA_SIZE];
> exec sql end declare section;
>
> exec sql connect to :conn_name; !! error
>
> - Conversion is not nee
[PROPOSAL]a new data type 'bytea' for ECPG
Hi, Hackers
# This is my first post.
I will try to implement a new data type 'bytea' for ECPG.
I think that the implementation is not complicated.
Does anyone need it ?
* Why do I need bytea ?
Currently, ECPG program can treat binary data for bytea column with 'char' type
of C language, but it must convert from/to escaped format with PQunescapeBytea/
PQescapeBytea(). It forces users to add an unnecessary code and to pay cost for
the conversion in runtime.
# My PoC will not be able to solve output conversion cost.
I think that set/put data for host variable should be more simple.
The following is an example of Oracle Pro *C program for RAW type column.
VARCHAR raw_data[20];
/* preprocessed to the following
* struct
* {
*unsigned short len;
*unsigned char arr[20];
* } raw_data;
*/
raw_data.len = 10;
memcpy(raw_data.arr, data, 10);
see also:
https://docs.oracle.com/cd/E11882_01/appdev.112/e10825/pc_04dat.htm#i23305
In ECPG, varchar host variable cannot be used for bytea because it cannot treat
'\0' as part of data. If the length is set to 10 and there is '\0' at 3rd byte,
ecpglib truncates 3rd byte and later at the following:
[src/interfaces/ecpg/ecpglib/execute.c]
ecpg_store_input(const int lineno, const bool force_indicator, const struct
:
switch (var->type)
:
case ECPGt_varchar:
if (!(newcopy = (char *) ecpg_alloc(variable->len + 1, lineno)))
return false;
!! strncpy(newcopy, variable->arr, variable->len);
newcopy[variable->len] = '\0';
I also think that the behavior of varchar host variable should not be changed
because of compatibility.
Therefore, a new type of host variable 'bytea' is needed.
Since ecpglib can distinguish between C string and binary data, it can send
binary data to backend directly by using 'paramFormats' argument of
PQexecParams().
Unfortunately, the conversion of output data cannot be omitted in ecpglib
because
libpq doesn't provide like 'paramFormats'.
('resultFormat' means that *all* data from backend is formatted by binary or
not.)
PQexecParams(PGconn *conn,
const char *command,
int nParams,
const Oid *paramTypes,
const char *const *paramValues,
const int *paramLengths,
!! const int *paramFormats,
int resultFormat)
* How to use new 'bytea' ?
ECPG programmers can use almost same as 'varchar' but cannot use as names.
(e.g. connection name, prepared statement name, cursor name and so on)
- Can use in Declare Section.
exec sql begin declare section;
bytea data1[512];
bytea data2[DATA_SIZE]; /* can use macro */
bytea send_data[DATA_NUM][DATA_SIZE]; /* can use two dimensional array */
bytea recv_data[][DATA_SIZE]; /* can use flexible array */
exec sql end declare section;
- Can *not* use for name.
exec sql begin declare section;
bytea conn_name[DATA_SIZE];
exec sql end declare section;
exec sql connect to :conn_name; !! error
- Conversion is not needed in user program.
exec sql begin declare section;
bytea send_buf[DATA_SIZE];
bytea recv_buf[DATA_SIZE - 13];
int ind_recv;
exec sql end declare section;
exec sql create table test (data1 bytea);
exec sql truncate test;
exec sql insert into test (data1) values (:send_buf);
exec sql select data1 into :recv_buf:ind_recv from test;
/* ind_recv is set to 13. */
* How to preprocess 'bytea' ?
'bytea' is preprocessed almost same as varchar.
The following is preprocessed to the next.
exec sql begin declare section;
bytea data[DATA_SIZE];
bytea send_data[DATA_NUM][DATA_SIZE];
bytea recv_data[][DATA_SIZE];
exec sql end declare section;
struct bytea_1 {int len; char arr[DATA_SIZE]} data;
struct bytea_2 {int len; char arr[DATA_SIZE]} send_data[DATA_NUM];
struct bytea_3 {int len; char arr[DATA_SIZE]} *recv_data;
Thank you for your consideration.
Regards
Ryo Matsumura
