show-stopper bug in pgsql_prepare()

[GitMensch]

This function eats any quotation -

gixsql/runtime/libgixsql-pgsql/DbInterfacePGSQL.cpp

Lines 265 to 294 in 081674b

	std::string pgsql_prepare(const std::string& sql)
	{
	int n = 1;
	std::vector<std::string> items;
	std::vector<std::string> out_items;
	split_in_args(items, sql, true);
	for (auto item : items) {
	if (starts_with(item, "\"") || item.back() == '\'') {
	out_items.push_back(item);
	continue;
	}
	size_t pos = item.find('?');
	if (pos == std::string::npos) {
	out_items.push_back(item);
	continue;
	}
	std::string s;
	for (auto c : item) {
	if (c == '?')
	s += ("$" + std::to_string(n++));
	else
	s += c;
	}
	out_items.push_back(s);
	}
	return vector_join(out_items, ' ');
	}

Input: SET APPLICATION_NAME TO "Identifier1 Identifier2 Identifier3"
Output: SET APPLICATION_NAME TO Identifier1 Identifier2 Identifer3
Result from DB:

ERROR: syntax error at or near "Identifier2"
LINE 1: SET APPLICATION_NAME TO Identifer1 Identifier2 Identife...

Workaround for this specific statement: SET APPLICATION_NAME TO "Identifier1-Identifier2-Identifier3", but that's unlikely to be possible in every other case (and not that easy to look for in the complete code).

[mridoni]

Yes, that's the problem with the string constants I was talking about in #21 (comment), I am working on it.

Thanks

[mridoni]

This should now be fixed in the internal repository, this

      EXEC SQL
      SET APPLICATION_NAME TO "Identifier1 Identifier2 Identifier3"
      END-EXEC.

is emitted as:

GIXSQL 01  SQ0002.
GIXSQL     02  FILLER PIC X(0061) VALUE "SET APPLICATION_NAME TO ""Ide"
GIXSQL  &  "ntifier1 Identifier2 Identifier3""".
GIXSQL     02  FILLER PIC X(1) VALUE X"00".

[GitMensch]

Thank you for the update. Should I try to test with the current version or is a bugfix version that includes #88 and maybe even VARCHAR2 likely until mid of next week (I'm on a longer vacation afterwards)?

[mridoni]

I don’t think I can manage a “full” release, a pre-release containing only the updated source and an autoconf-enabled tarball is more likely (but not sure). I already made a few modifications for #88 (letting “smart” cursor init be the default) but I could not reproduce your problem (my test cases pass but there is obviously something missing); a minimal test case, if you have time, might help.

The new EXEC SQL VAR stuff could also be coming in the same timeframe and pre-release (still: if possible, not 100% sure) but since it touches a core part of the runtime, I am a little afraid of regressions, so I will have to see how it behaves.

Thanks.

[GitMensch]

Looks like you've fixed a different issue here - in the compiler.
My case - with the referenced code, which is unchaged in 1.0.18dev1 - is still broken: dynamic prepared statements with a stack trace of

#0  pgsql_prepare (...) at libgixsql-pgsql/DbInterfacePGSQL.cpp:260
#1  DbInterfacePGSQL::prepare (...) at libgixsql-pgsql/DbInterfacePGSQL.cpp:297
#2  GIXSQLPrepareStatement (...) at libgixsql/gixsql.cpp:928

Sample from above: the prepared statement variable in COBOL contains:
SET APPLICATION_NAME TO "Identifier1 Identifier2 Identifier3" (there was a workaround of replacing spaces to underscores here)

Sample 2 (which there is no workaround for):
SELECT MAX(VAL) FROM TAB WHERE TABART NOT IN ('AAAAA ' ,'BBBBBB ' ,'CCCCCCCC' ,'DDDD ' ,'EEEEEEEE' )

and an even more strange output from pgsql_prepare:

SELECT MAX(VAL) FROM TAB WHERE TABART NOT IN ('AAAAA ' , BBBBB ' , CCCCCCC' ,'DDDD ' , EEEEEEE' )

[GitMensch]

Quick Hack - which possibly chrashes something else and most likely dynamic parameters:

-	std::string prepared_sql = pgsql_prepare(sql);
+	std::string prepared_sql = std::string(sql);

possibly could improved to still support dynamic parameters with some "if find '?' ... but fixing pgsql_prepare would be the correct thing in any case.

[mridoni]

Looks like you've fixed a different issue here - in the compiler.

You are right, I will check this.

[mridoni]

I rewrote pgsql_prepare, this is fixed in the internal repository. BTW: this function just transforms all anonymous parameter placeholders into numbered ones, as PostgreSQL requires. It adds some minimal "cross-platform behaviour" (e.g. you don't need to modify the prepared statements when going from ODBC to PostgreSQL) but it could be removed if deemed unnecessary.

std::string pgsql_prepare(const std::string& sql)
{
	int n = 1;
	bool in_single_quoted_string = false;
	bool in_double_quoted_string = false;
	std::string out_sql;

	for (auto itc = sql.begin(); itc != sql.end(); ++itc) {
		char c = *itc;

		switch (c) {
			case '"':
				out_sql += c;
				in_double_quoted_string = !in_double_quoted_string;
				continue;

			case '\'':
				out_sql += c;
				in_single_quoted_string = !in_single_quoted_string;
				continue;

			case '?':
				if (!in_single_quoted_string && !in_double_quoted_string)
					out_sql += ("$" + std::to_string(n++));
				else 
					out_sql += c;
				continue;

			default:
				out_sql += c;

		}
	}
	
	return out_sql;
}

[GitMensch]

That looks to be fixed, so closing.