Re: Add LINE: hint when schemaname.typename is a non-existent schema

From: Ryan Kelly <rpkelly22(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Add LINE: hint when schemaname.typename is a non-existent schema
Date: 2015-02-08 18:15:17
Message-ID: CAHUie27wo3gD3v54_ixevd-vg-d1+CoQXnQ9GJf5xyZwDg5ExA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Feb 3, 2015 at 11:10 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Ryan Kelly <rpkelly22(at)gmail(dot)com> writes:
>> The attached patch adds a LINE: ... hint when schemaname.typename
>> results in a schema which does not exist. I came across this when a
>> missing comma in a SELECT list resulted in an error without a location
>> in a query a few thousand lines long.
>
> I think this is a good problem to attack, but the proposed patch seems
> a bit narrow and brute-force. Surely this isn't the only place where
> we're missing an errposition pointer on a "no such schema" error?

I managed to find four code paths that failed to report an error position if
the schema did not exist:
- Schema-qualified types (e.g. select schemaname.typename 'foo')
- Schema-qualified operators (e.g. select 1 operator(schemaname.+) 1)
- Schema-qualified table names in CREATE TABLE (e.g. create table
schemaname.tablename (colname integer))
- Schema-qualified function calls (e.g. select schemaname.funcname())

> It's probably not reasonable to add a pstate argument to
> LookupExplicitNamespace itself, given that namespace.c isn't part
> of the parser. But you could imagine adding an interface function
> in the parser that calls LookupExplicitNamespace and then throws a
> position-aware error on failure, and then making sure that all schema
> lookups in the parser go through that.

While searching for other instances of this problem, I found that using
a schema that does not exist in conjunction with a collation reported the
position. That code uses setup_parser_errposition_callback and the
documentation for that function seems to indicate that the usage of it in my
newly attached patch are consistent. I do not know, however, if this is the
cleanest approach or if I should actually create a function like
validate_explicit_namespace to handle this (and I'm also not sure where
such a function should live, if I do create it).

-Ryan Kelly

Attachment Content-Type Size
missing_type_schema_hint_v2.patch application/octet-stream 7.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Abhijit Menon-Sen 2015-02-08 18:33:59 Re: What exactly is our CRC algorithm?
Previous Message Robert Haas 2015-02-08 17:33:28 Re: Parallel Seq Scan