Thread: Re: [HACKERS] pgbench - allow to store select results intovariables

Re: [HACKERS] pgbench - allow to store select results intovariables

From
Tatsuo Ishii
Date:
>> 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



Re: [HACKERS] pgbench - allow to store select results intovariables

From
Fabien COELHO
Date:
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.



Re: [HACKERS] pgbench - allow to store select results intovariables

From
Tatsuo Ishii
Date:
> 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



Re: [HACKERS] pgbench - allow to store select results intovariables

From
Fabien COELHO
Date:
>> 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.



Re: [HACKERS] pgbench - allow to store select results intovariables

From
Fabien COELHO
Date:
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

Re: [HACKERS] pgbench - allow to store select results intovariables

From
Fabien COELHO
Date:
> 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

Re: [HACKERS] pgbench - allow to store select results intovariables

From
Fabien COELHO
Date:
> 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

Re: [HACKERS] pgbench - allow to store select results intovariables

From
Tatsuo Ishii
Date:
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



Re: [HACKERS] pgbench - allow to store select results intovariables

From
Fabien COELHO
Date:
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

Re: [HACKERS] pgbench - allow to store select results intovariables

From
Tatsuo Ishii
Date:
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



Re: [HACKERS] pgbench - allow to store select results intovariables

From
Fabien COELHO
Date:
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

Re: [HACKERS] pgbench - allow to store select results intovariables

From
Tatsuo Ishii
Date:
> 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



Re: [HACKERS] pgbench - allow to store select results intovariables

From
Fabien COELHO
Date:
>> 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.



Re: [HACKERS] pgbench - allow to store select results into variables

From
Tom Lane
Date:
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



Re: [HACKERS] pgbench - allow to store select results intovariables

From
Fabien COELHO
Date:
>> 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.