RE: Problem during Windows service start
Hi, Thank you for picking up this and I'm sorry for delay reply. >> If wait_for_postmaster returns POSTMASTER_STILL_STARTING will it >> be correct to set the status of windows service to SERVICE_START_PENDING ? Yes, I think this is the best. Currently, I do not have good solution to change the status from SERVICE_START_PENDING to SERVICE_RUNNING after recovery is completed. >> I would like to take this up if no one is working on this. Thank you for your proposing! I would like to ask if possible. I do not have much time to think about this topic now... Regards, Higuchi
RE: Problem during Windows service start
Hi,
+ case POSTMASTER_STILL_STARTING:
+ write_eventlog(EVENTLOG_ERROR_TYPE, _("Timed out waiting for
server startup\n"));
+ pgwin32_SetServiceStatus(SERVICE_START_PENDING);
+ return;
Could this patch solve first post's problem [1] ?
I think no one could change the service status to SERVICE_RUNNING even if the
server has been started properly after timeout is occurred.
In 9.6 or earlier, the main use case where the problem is occurred is when
timeout is occured because of long time recovery. Even if recovery takes a lot
of time and timeout is ocurred, recovery continues in background. In this case,
I want to set the status to SERVICE_RUNNING after recovery is completed.
In 10 or later, I understand wait_for_postmaster does not wait until recovery
is completed, so I think this problem rarely occurs in PG 10 or later.
[1]
https://www.postgresql.org/message-id/99C4246E73ED1B478BBB9671718426203E37F485@G01JPEXMBKW03
Regards,
Daisuke, Higuchi
RE: [Bug Fix]ECPG: cancellation of significant digits on ECPG
From: Dmitry Dolgov [mailto:9erthali...@gmail.com] > Thanks for the patches. Unfortunately, judging from the cfbot.cputube.org they > can't be applied anymore to the current master, could you please rebase them? Thank you for checking! I rebased patches on the current master, so I attach them. Regards, Daisuke Higuchi 002_ecpg_numeric_test_v2.patch Description: 002_ecpg_numeric_test_v2.patch 001_ecpg_numeric_bugfix_v2.patch Description: 001_ecpg_numeric_bugfix_v2.patch
RE: [Bug Fix]ECPG: cancellation of significant digits on ECPG
From: Dmitry Dolgov [mailto:9erthali...@gmail.com] >Also what's strange for me is that after applying your patches I still got the >same output, not sure why: Hmm... In my environment, the output was changed. I also updated regression test and cfbot.cputube.org reports no problem now. Do you use the sample application which I attached on the first posted mail? If not, could you please share the details? Regards, Daisuke Higuchi
RE: [Bug Fix]ECPG: cancellation of significant digits on ECPG
From: Tom Lane [mailto:t...@sss.pgh.pa.us] >I took a quick look at this. Thank you for review. >So I think that we ought to unconditionally make the sqlda value's digit >buffer look just like the one we're copying, even when ndigits = 0, >which just requires removing the tests on ndigits. I agree with you. Seeing this thread[1], 'if (ndigits = 0)' was introduced only to avoid memcpy() crash. I do not know this solution was best or not, but no crash occurs in the current version. So, I also think 'if (ndigits = 0)' should be removed. >In short, the attached. Thank you for updating. This patch looks good to me. [1] https://www.postgresql.org/message-id/4EC825F3.5080504%40cybertec.at Regards, Daisuke Higuchi
[Bug Fix] ECPG: could not use some CREATE TABLE AS syntax
Hi, I found some "CREATE TABLE ... AS ... " syntaxes could not be used in ECPG. [PROBLEM] First, ECPG command is failed when the source code (*.pgc) has "IF NOT EXISTS". --- EXEC SQL CREATE TABLE IF NOT EXISTS test_cta AS SELECT * FROM test; --- Second, ECPG command is succeeded when the source code (*.pgc) has following embedded SQL. However, created c program has no "WITH NO DATA". -- EXEC SQL CREATE TABLE test_cta AS SELECT * FROM test WITH NO DATA; -- [Investigation] In my investigation, parse.pl ignore type CreateAsStmt of gram.y and CreateAsStmt is defined in ecpg.trailer. ECPG use ecpg.trailer's CreateAsStmt. However, ecpg.trailer lacks some syntaxes. I feel ignoring type CreateAsStmt of gram.y is strange. Seeing ecpg.trailer, it seems that ECPG wanted to output the message "CREATE TABLE AS cannot specify INTO" but is this needed now? In view of the maintenance, ECPG should use not ecpg.trailer's definition but gram.y's one. I attached the patch for this and I will register this for next CF. Regards, Daisuke Higuchi FIX_ECPG_CREATE_TABLE_AS_v1.patch Description: FIX_ECPG_CREATE_TABLE_AS_v1.patch
RE: [Bug Fix] ECPG: could not use some CREATE TABLE AS syntax
Hi, Meskes-san,
Thanks for your response!
> I beg to disagree, or I don't understand. Why would ecpg's changes to
> the statement be wrong nowadays?
I might confuse you, but it does not mean that it is wrong to reject CREATE
TABLE AS ... INTO ... syntax in ECPG.
My goal is to accept syntax which is currently rejected by ECPG. To realize
that, I am considering following two ways:
(a) new syntax of create as statement should be added to ECPG
(b) make ECPG to use not ecpg.trailer but gram.y in the syntax of create as
statement
In (a), we need to keep similar codes in both ecpg.trailer and gram.y. Also, if
the syntax of create as statement will be changed in the future, there is a
possibility that it will not be reflected in ECPG like this bug. Therefore, I
thought that (b) was better and created a patch. And, in order to make it the
simplest code, some SQL which is rejected in current ECPG is accepted in my
patch's ECPG.
> The statement CREATE TABLE a AS SELECT * INTO test FROM a; is accepted
> with your patch, but it is not accepted in current ecpg nor is it accepted
> by the backend when you execute it through ecpg.
Indeed, CREATE TABLE a AS SELECT * INTO test FROM a; is accepted in my patch's
ECPG, but the backend always reject, but which SQL should be rejected in both
ECPG and the backend? Following inappropriate SQL are accepted in ECPG but
rejected by the backend (I am wondering why only CREATE TABLE .. AS .. INTO is
rejected and other inappropriate SQL are accepted in current ECPG. ).
- EXEC SQL delete from test where c1 = (select into c2 from test2);
From the viewpoint of compatibility, if (b) is not good, I will consider (a)
solution like following:
diff --git a/src/interfaces/ecpg/preproc/ecpg.trailer
b/src/interfaces/ecpg/preproc/ecpg.trailer
@@ -34,7 +34,14 @@ CreateAsStmt: CREATE OptTemp TABLE create_as_target AS
{FoundInto = 0;} SelectSt
if (FoundInto == 1)
mmerror(PARSE_ERROR, ET_ERROR, "CREATE TABLE AS
cannot specify INTO");
- $$ = cat_str(6, mm_strdup("create"), $2,
mm_strdup("table"), $4, mm_strdup("as"), $7);
+ $$ = cat_str(7, mm_strdup("create"), $2, mm_strdup("table"),
$4, mm_strdup("as"), $7, $8);
+ }
+| CREATE OptTemp TABLE IF_P NOT EXISTS create_as_target AS
{FoundInto = 0;} SelectStmt opt_with_data
+ {
+ if (FoundInto == 1)
+ mmerror(PARSE_ERROR, ET_ERROR, "CREATE TABLE AS
cannot specify INTO");
+
+ $$ = cat_str(7, mm_strdup("create"), $2, mm_strdup("table if
not exists"), $7, mm_strdup("as"), $10, $11);
}
;
I also want to hear your opinion. I will change my opinion flexibly.
Regards,
Daisuke, Higuchi
RE: [Bug Fix] ECPG: could not use some CREATE TABLE AS syntax
Hi, Meskes-san > Yes, I fully understand that. However, in doing so you accept > statements that the backend later on rejects. The sole reason for > the big ecpg grammar is to prevent those cases whenever possible. Ok, I agree with you. > > I also want to hear your opinion. I will change my opinion flexibly. > I agree that this the way to go. I updated and attached the patch. As I show in previous post, this version is that "IF NOT EXISTS" keyword and variable for "WITH NO DATA" are added to ecpg.trailer. Regards, Daisuke, Higuchi FIX_ECPG_CREATE_TABLE_AS_v2.patch Description: FIX_ECPG_CREATE_TABLE_AS_v2.patch
RE: [Bug Fix] ECPG: could not use some CREATE TABLE AS syntax
Hi, Meskes-san > Thank you, committed. Thank you! By the way, I have found another bugs which are related to ECPG , so I will post that later. Regards, Daisuke, Higuchi
[Bug Fix] ECPG: could not use set xxx to default statement
Hi,
I found ECPG's bug which SET xxx = DEFAULT statement could not be used.
[PROBLEM]
When the source code (*.pgc) has "EXEC SQL set xxx = default;", created c
program by ECPG has no " = default".
For example, "EXEC SQL set search_path = default;" in *.pgc will be converted
to "{ ECPGdo(__LINE__, 0, 1, NULL, 0, ECPGst_normal, "set search_path",
ECPGt_EOIT, ECPGt_EORT);}" in c program.
[Investigation]
gram.y lacks ";" in the end of section "generic_set", so preproc.y's syntax is
broken.
src/backend/parser/gram.y
---
generic_set:
| var_name '=' DEFAULT
{
VariableSetStmt *n =
makeNode(VariableSetStmt);
n->kind = VAR_SET_DEFAULT;
n->name = $1;
$$ = n;
}
set_rest_more: /* Generic SET syntaxes: */
---
src/interfaces/ecpg/preproc/preproc.y
---
generic_set:
| var_name TO DEFAULT
{
$$ = cat_str(2,$1,mm_strdup("to default"));
}
| var_name '=' DEFAULT set_rest_more:
generic_set
{
$$ = $1;
}
---
I attached the patch which has ";" in the end of section "generic_set" and
regression.
Regards,
Daisuke, Higuchi
FIX_ECPG_SET_TO_DEFAULT_v1.patch
Description: FIX_ECPG_SET_TO_DEFAULT_v1.patch
RE: [Bug Fix] ECPG: could not use set xxx to default statement
Hi, > I think we need to fix that script to either cope with missing semicolons, > or at least complain about them. Too tired to look into how, right now. I attached the patch which cope with missing semicolons. Previous parse.pl find semicolon and dump data to buffer. When attached patch's parse.pl find new tokens before finding a semicolon, it also dumps data to buffer. Regards, Daisuke, Higuchi FIX_ECPG_SET_TO_DEFAULT_v2.patch Description: FIX_ECPG_SET_TO_DEFAULT_v2.patch
RE: Problem during Windows service start
Hi, This thread is inactive, but I want to solve this problem. I think this problem rarely occurs in 10 or later version because of commit [1]. Because "pg_ctl start -w" wait for only PID file creation. It means that timeout is not occurred even if crash recovery takes a lot of times. However, 9.6 or earlier still wait for long time recovery complete. > How do you propose to fix it? I think there are two solutions. One solution is that status of Windows Service should be changed to "SERVICE_RUNNING" even if timeout is occurred because of long time recovery. I attached the patch of this solution. Another solution is to backport commit [1] to 9.6 or earlier version. However this fix change the content of PID file, so incompatible change, I think. I would appreciate it if you give any comments. [1] https://github.com/postgres/postgres/commit/f13ea95f9e473a43ee4e1baeb94daaf83535d37c Regards, Daisuke, Higuchi windows_service_bug_fix.patch Description: windows_service_bug_fix.patch
RE: Problem during Windows service start
Michael Paquier wrote:
> You should register this patch to the next commit fest in the section for bug
> fixes to not lose sight of it;
Thank you for picking up my post. I registered to the next CF.
> I haven't put much thoughts into what you propose here, but this status
> message is not really helpful for the user.
Thank you for review. I attached the updated patch. There are some reasons for
server startup failing, so I changed the message like following:
+ case PQPING_NO_RESPONSE:
+ case PQPING_NO_ATTEMPT:
+ write_eventlog(EVENTLOG_ERROR_TYPE, _("Server
startup failed. Examine the log output.\n"));
+ pgwin32_SetServiceStatus(SERVICE_STOPPED);
Regards,
Daisuke, Higuchi
windows_service_bug_fix_v2.patch
Description: windows_service_bug_fix_v2.patch
[Bug Fix]ECPG: cancellation of significant digits on ECPG
Hi,
Currently our customer uses PostgreSQL 9.6 and hits ECPG's bug during using
numeric data type by SQLDA.
I confirmed that this problem is occurred on master and 9.6 latest branch.
PROBLEM
---
When the integer part of numeric data type is "0", cancellation of significant
digits is occurred.
For example, I expect "0.12345", but I got "0.12340". When I expect "0.01234",
I got "0.01200"
I attached the sample application code to reproduce this problem.
CAUSE
---
When copy the data of numeric data, the size is wrong.
"src/interfaces/ecpg/ecpglib/sqlda.c" has problem.
ecpg_set_native_sqlda(int lineno, struct sqlda_struct ** _sqlda, const PGresult
*res, int row, enum COMPAT_MODE compat)
{
...
if (num->ndigits)
{
ecpg_sqlda_align_add_size(next_offset, sizeof(int), num->ndigits + 1,
&offset, &next_offset);
memcpy((char *) sqlda + offset, num->buf, num->ndigits + 1);
((numeric *) sqlda->sqlvar[i].sqldata)->buf = (NumericDigit *) sqlda +
offset;
((numeric *) sqlda->sqlvar[i].sqldata)->digits = (NumericDigit *) sqlda
+ offset + (num->digits - num->buf);
}
...
When numeric data is "0.12345", num->buf has "0 0 1 2 3 4 5" and num->digits
has "1 2 3 4 5".
num->ndigits has the number of digits which is or later "1", it means 5.
In this code, currently copy "num->ndigits + 1" as size of numeric data.
As a result, (char *) sqlda + offset has "0 0 1 2 3 4", not "0 0 1 2 3 4 5".
So, "num->digits - num->buf + num->ndigits" should be copied.
FIX
---
Above source code should be fixed and other similar bugs are fixed too.
I attached patches for bug fix and regression test for master branch.
I hope this bug fix will be backport to other versions.
Regards,
Daisuke Higuchi
001_ecpg_numeric_bugfix_v1.patch
Description: 001_ecpg_numeric_bugfix_v1.patch
numeric_sample_test.pgc
Description: numeric_sample_test.pgc
002_ecpg_numeric_test_v1.patch
Description: 002_ecpg_numeric_test_v1.patch
RE: [Bug Fix]ECPG: cancellation of significant digits on ECPG
Hi,
I have a question which is related to numeric data type bugs on ECPG sqlda.
Currently, I think following functions have a problem when using numeric data
type on sqlda.
Please see the details of problem from the mail I sent before.
[src/ecpg/ecpglib/sqlda.c]
- ecpg_set_native_sqlda()
- sqlda_common_total_size()
- ecpg_set_compat_sqlda()
However, I think some codes which have problem in ecpg_set_compat_sqlda() are
not used.
On ecpg_set_compat_sqlda(), there are own cases for numeric and decimal data
type.
-
ecpg_set_compat_sqlda(int lineno, struct sqlda_compat **_sqlda, const PGresult
*res, int row, enum COMPAT_MODE compat)
{
for (i = 0; i < sqlda->sqld; i++)
{
switch (sqlda->sqlvar[i].sqltype)
{
case ECPGt_decimal:
ecpg_sqlda_align_add_size(offset, sizeof(double),
sizeof(double), &offset, &next_offset);
case ECPGt_numeric:
ecpg_sqlda_align_add_size(next_offset, sizeof(int), num->ndigits
+ 1, &offset, &next_offset);
-
ecpg_set_compat_sqlda() is used only when INFORMIX_MODE(stmt->compat) is true.
-
bool
ecpg_process_output(struct statement *stmt, bool clear_result)
{
if (INFORMIX_MODE(stmt->compat))
{
sqlda_new = ecpg_build_compat_sqlda(stmt->lineno,
stmt->results, i, stmt->compat);
ecpg_set_compat_sqlda(stmt->lineno, _sqlda, stmt->results, i,
stmt->compat);
}
-
However, numeric data type could be changed to decimal by
ecpg_build_compat_sqlda() in this case.
-
ecpg_build_compat_sqlda(int line, PGresult *res, int row, enum COMPAT_MODE
compat)
{
sqlda->sqlvar[i].sqltype = sqlda_dynamic_type(PQftype(res, i),
compat);
sqlda_dynamic_type(Oid type, enum COMPAT_MODE compat)
{
switch (type)
{
case NUMERICOID:
return INFORMIX_MODE(compat) ? ECPGt_decimal :
ECPGt_numeric;
-
Could we remove some codes for numeric in ecpg_set_compat_sqlda()?
Regards,
Daisuke, Higuchi
stat() on Windows might cause error if target file is larger than 4GB
Hi, This mail is about following bug report post: https://www.postgresql.org/message-id/flat/153442391458.1505.9181095584291689853%40wrigleys.postgresql.org When pg_dump has '--format=directory' option and the dump file size become 4GB over, the strange error message 'Unknown error' will be output. This is because _stat64i32() is used for stat() on Windows, I think. Seeing following URL, _stat64i32() could use 32 bit, it means 4GB is max size. https://msdn.microsoft.com/en-us/library/14h5k7ff.aspx When I create the simple application to use stat() on VS2013, stat() could not deal with 4GB file and failed with errno=132. I think the pg_dump occurs EOVERFLOW, but Windows' errno.h does not have this errno, so 'Unknown error' is output. So, pgwin32_safestat() should be changed to solve this problem. Do you have any idea or comments? Regards, Daisuke Higuchi
RE: stat() on Windows might cause error if target file is larger than 4GB
Michael-san,
From: Michael Paquier [mailto:mich...@paquier.xyz]
>Does something like the patch attached help?
>This makes sure that st_size is set correctly for files with a size larger
>than 4GB.
Thank you for creating patch, but this does not solve current problem.
Of cause setting wrong size to st_size is problem,
I think the cause of this problem is stat()'s return value (=-1).
In pgwin32_safestat(), if stat() try to deal with files with a size larger than
4GB,
the return value is -1. So, pgwin32_safestat() exits before calculating
buf->st_size.
pgwin32_safestat(const char *path, struct stat *buf)
{
int r;
WIN32_FILE_ATTRIBUTE_DATA attr;
r = stat(path, buf);
if (r < 0)
{
...
return r;
}
...
buf->st_size = attr.nFileSizeLow;
return 0;
}
So, attached patch help me and strange message disappeared,
but I ignore the impact of this for others now.
Regards,
Daisuke, Higuchi
win32-stat-remove-return.patch
Description: win32-stat-remove-return.patch
