On 20.01.22 08:37, tanghy.fnst@fujitsu.com wrote:
>> 1. The downcasing logic in the patch bears very little resemblance
>> to the backend's actual downcasing logic, which can be found in
>> src/backend/parser/scansup.c's downcase_identifier(). Notably,
>> the patch's restriction to only convert all-ASCII strings seems
>> indefensible, because that's not how things really work. I fear
>> we can't always exactly duplicate the backend's behavior, because
>> it's dependent on the server's locale and encoding; but I think
>> we should at least get it right in the common case where psql is
>> using the same locale and encoding as the server.
> Thanks for your suggestion, I removed ASCII strings check function
> and added single byte encoding check just like downcase_identifier.
> Also added PGCLIENTENCODING setting in the test script to make
> test cases pass.
> Now the patch supports tab-completion with none-quoted upper characters
> available when client encoding is in single byte.
The way your patch works now is that the case-insensitive behavior you
are implementing only works if the client encoding is a single-byte
encoding. This isn't what downcase_identifier() does;
downcase_identifier() always works for ASCII characters. As it is, this
patch is nearly useless, since very few people use single-byte client
encodings anymore. Also, I think it would be highly confusing if the
tab completion behavior depended on the client encoding in a significant
way.
Also, as I had previously suspected, your patch treats the completion of
enum labels in a case-insensitive way (since it all goes through
_complete_from_query()), but enum labels are not case insensitive. You
can observe this behavior using this test case:
+check_completion("ALTER TYPE enum1 RENAME VALUE 'F\t\t", qr|foo|, "FIXME");
+
+clear_line();
You should devise a principled way to communicate to
_complete_from_query() whether it should do case-sensitive or
-insensitive completion. We already have COMPLETE_WITH() and
COMPLETE_WITH_CS() etc. to do this in other cases, so it should be
straightforward to adapt a similar system.