From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)postgreSQL(dot)org |
Subject: | So do we really *need* those substring() ops in tab-completion queries? |
Date: | 2010-01-03 01:21:35 |
Message-ID: | 22832.1262481695@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
This evening's argument about DO completion caused me to look a bit
closer at tab-complete.c than I ever had before. I am now wondering
exactly why we bother with all the logic like
" WHERE substring(pg_catalog.quote_ident(rolname),1,%d)='%s'"
in the SQL queries that it issues. It appears to me that this code is:
1. Wrong. The value substituted for %d is strlen() of the partial word
it wants to match; but the backend is going to count substring's
argument in characters not bytes. So it looks to me like this code
fails entirely when dealing with names containing multibyte characters.
I'm not in a very good position to confirm that right now, but perhaps
someone can try it.
2. Unnecessary. The loop at the bottom of _complete_from_query applies
pg_strncasecmp anyway to filter out query results that don't match the
supplied partial word. (BTW, why is this pg_strncasecmp and not just
strncmp? The SQL-level filter will have got rid of non-exact matches,
so that's useless, and possibly wrong depending on locale issues.)
3. Inefficient. It seems likely to me that filtering on the prefix on
the backend side isn't going to be more efficient than doing it on the
client side, except maybe in the schema-name cases. If the conditions
were phrased in a way that made them indexable, they might be worth the
trouble --- but they aren't. In the worst case where we're asked for
completions from a zero-length string, the backend-side substring ops
are certainly pure overhead.
Comments?
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2010-01-03 01:51:22 | Re: invalid UTF-8 via pl/perl |
Previous Message | Peter Eisentraut | 2010-01-03 01:04:10 | Re: PATCH: Add hstore_to_json() |