Thread: Re: [HACKERS] pgbench - allow to store select results intovariables
>> Having said all that, I think we're at the point in the commitfest >> where if there's any design question at all about a patch, it should >> get booted to the next cycle. We are going to have more than enough >> to do to stabilize what's already committed, we don't need to be >> adding more uncertainty. > > Ok, I will move the patch to the next cf. Done. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Hello Tatsuo, >> Ok, I will move the patch to the next cf. > > Done. If I understand correctly, the patch is moved because of the unrelated issue that variables cannot be utf8 in pgbench, and it is a condition to consider this patch that existing pgbench variables (set with \set) can be utf8? -- Fabien.
> If I understand correctly, the patch is moved because of the unrelated > issue that variables cannot be utf8 in pgbench, and it is a condition > to consider this patch that existing pgbench variables (set with \set) > can be utf8? I'm not sure if it is "unrelated" because the new feature relies on existing pgbench variable infrastructure. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
>> If I understand correctly, the patch is moved because of the unrelated >> issue that variables cannot be utf8 in pgbench, and it is a condition >> to consider this patch that existing pgbench variables (set with \set) >> can be utf8? > > I'm not sure if it is "unrelated" because the new feature relies on > existing pgbench variable infrastructure. Sure. I meant that the constraint on variable names exists before the patch and the patch is not related to variable names, but the patch is about variables, obviously. As "psql" variables can be utf8 and that the same scanner is used, but the variables values are not stritcly the same (they are typed in pgbench), I'm wondering whether the effort should be do share more code/abstraction between psql & pgbench or just adjust/replicate the needed small functions/code excerpts. -- Fabien.
Hello Tatsuo-san, >>> If I understand correctly, the patch is moved because of the unrelated >>> issue that variables cannot be utf8 in pgbench, and it is a condition >>> to consider this patch that existing pgbench variables (set with \set) >>> can be utf8? >> >> I'm not sure if it is "unrelated" because the new feature relies on >> existing pgbench variable infrastructure. > > Sure. I meant that the constraint on variable names exists before the patch > and the patch is not related to variable names, but the patch is about > variables, obviously. > > As "psql" variables can be utf8 and that the same scanner is used, but the > variables values are not stritcly the same (they are typed in pgbench), I'm > wondering whether the effort should be do share more code/abstraction between > psql & pgbench or just adjust/replicate the needed small functions/code > excerpts. As the variable infrastructures are pretty different between psql & pgbench (typed vs untyped values, sorted array vs linked list data structure, no hook vs 2 hooks, name spaces vs no such thing...), I have chosen the simplest option of just copying the name checking function and extending the lexer to authorize non-ascii letters, so that psql/pgbench would accept the same variable names with the same constraint about encodings. See patch attached & test script. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
> As the variable infrastructures are pretty different between psql & pgbench > (typed vs untyped values, sorted array vs linked list data structure, no hook > vs 2 hooks, name spaces vs no such thing...), I have chosen the simplest > option of just copying the name checking function and extending the lexer to > authorize non-ascii letters, so that psql/pgbench would accept the same > variable names with the same constraint about encodings. > > See patch attached & test script. Argh, I'm jet-lagged, wrong patch suffix... Here it is with the right suffix. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
> As the variable infrastructures are pretty different between psql & pgbench > (typed vs untyped values, sorted array vs linked list data structure, no hook > vs 2 hooks, name spaces vs no such thing...), I have chosen the simplest > option of just copying the name checking function and extending the lexer to > authorize non-ascii letters, so that psql/pgbench would accept the same > variable names with the same constraint about encodings. > > See patch attached & test script. Argh, I should be asleep:-( Wrong patch suffix, wrong from, multiple apology:-( Here it is again. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Fabien, >> As the variable infrastructures are pretty different between psql & >> pgbench (typed vs untyped values, sorted array vs linked list data >> structure, no hook vs 2 hooks, name spaces vs no such thing...), I >> have chosen the simplest option of just copying the name checking >> function and extending the lexer to authorize non-ascii letters, so >> that psql/pgbench would accept the same variable names with the same >> constraint about encodings. >> >> See patch attached & test script. > > Argh, I'm jet-lagged, wrong patch suffix... Here it is with the right > suffix. Thank you for the patch. I tested a little bit and found that it does not allow value replacement against non ascii variables in given SQL statements . Is it intentional? If not, I think you need to fix parseVariable() as well. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Hello Tatsuo-san, > Thank you for the patch. I tested a little bit and found that it does > not allow value replacement against non ascii variables in given SQL > statements . Is it intentional? No, this is a bug. > If not, I think you need to fix parseVariable() as well. Indeed. Here is v2. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Fabien, > Hello Tatsuo-san, > >> Thank you for the patch. I tested a little bit and found that it does >> not allow value replacement against non ascii variables in given SQL >> statements . Is it intentional? > > No, this is a bug. > >> If not, I think you need to fix parseVariable() as well. > > Indeed. Here is v2. I think you'd better to change the following comments because there's no psqlscan.l or psqlscanslash.l in pgbench source tree. + * underscore. Keep this in sync with the definition of variable_char in + * psqlscan.l and psqlscanslash.l. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
Hello, > I think you'd better to change the following comments because there's > no psqlscan.l or psqlscanslash.l in pgbench source tree. > > + * underscore. Keep this in sync with the definition of variable_char in > + * psqlscan.l and psqlscanslash.l. Here is a v3 with a more precise comment. -- Fabien. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
> Hello, > >> I think you'd better to change the following comments because there's >> no psqlscan.l or psqlscanslash.l in pgbench source tree. >> >> + * underscore. Keep this in sync with the definition of >> variable_char in >> + * psqlscan.l and psqlscanslash.l. > > Here is a v3 with a more precise comment. Looks good to me. I have marked the patch status as "Ready for committer". Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp
>> Here is a v3 with a more precise comment. > > Looks good to me. I have marked the patch status as "Ready for > committer". Ok. Thanks. When/if committed, it might trigger a few rebase of other pending patches. I'll see about that then. -- Fabien.
Tatsuo Ishii <ishii@sraoss.co.jp> writes:
>> Here is a v3 with a more precise comment.
> Looks good to me. I have marked the patch status as "Ready for
> committer".
LGTM too. Pushed with a minor adjustment to make parseVariable()
have exactly the same test as valid_variable_name().
regards, tom lane
>> Looks good to me. I have marked the patch status as "Ready for >> committer". > > LGTM too. Pushed with a minor adjustment to make parseVariable() > have exactly the same test as valid_variable_name(). \set ありがとうございました 1 \set 谢谢 2 \set dankeschön 3 :-) -- Fabien.