On Wed, Jul 29, 2015 at 12:44 PM, Shulgin, Oleksandr < oleksandr.shul...@zalando.de> wrote:
> On Tue, May 12, 2015 at 12:25 AM, Alvaro Herrera <alvhe...@2ndquadrant.com > > wrote: > >> David Steele wrote: >> >> > I have reviewed and tested this patch and everything looks good to me. >> > It also looks like you added better coverage for schema DDL, which is a >> > welcome addition. >> >> Thanks -- I have pushed this now. >> > > Hi, > > I've tried compiling the 0003-ddl_deparse-extension part from > http://www.postgresql.org/message-id/20150409161419.gc4...@alvh.no-ip.org > on current master and that has failed because the 0002 part hasn't been > actually pushed (I've asked Alvaro off the list about this, that's how I > know the reason ;-). > > I was able to update the 0002 part so it applies cleanly (updated version > attached), and then the contrib module compiles after one minor change and > seems to work. > > I've started to look into what it would take to move 0002's code to the > extension itself, and I've got a question about use of printTypmod() in > format_type_detailed(): > > if (typemod >= 0) > *typemodstr = printTypmod(NULL, typemod, typeform->typmodout); > else > *typemodstr = pstrdup(""); > > Given that printTypmod() does psprintf("%s%s") one way or the other, > shouldn't we pass an empty string here instead of NULL as typname argument? > Testing shows this is a bug indeed, easily triggered by deparsing any type with typmod. My hope is to get this test module extended quite a bit, not only to >> cover existing commands, but also so that it causes future changes to >> cause failure unless command collection is considered. (In a previous >> version of this patch, there was a test mode that ran everything in the >> serial_schedule of regular regression tests. That was IMV a good way to >> ensure that new commands were also tested to run under command >> collection. That hasn't been enabled in the new test module, and I >> think it's necessary.) >> > > >> If anyone wants to contribute to the test module so that more is >> covered, that would be much appreciated. >> > > I'm planning to have a look at this part also. > While running deparsecheck suite I'm getting a number of oddly looking errors: WARNING: state: 42883 errm: operator does not exist: pg_catalog.oid = pg_catalog.oid This is caused by deparsing create view, e.g.: STATEMENT: create view v1 as select * from t1 ; ERROR: operator does not exist: pg_catalog.oid = pg_catalog.oid at character 52 HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. QUERY: SELECT * FROM pg_catalog.pg_rewrite WHERE ev_class = $1 AND rulename = $2 CONTEXT: PL/pgSQL function test_ddl_deparse() line 1 at FOR over SELECT rows The pg_rewrite query comes from ruleutils.c, while ddl_deparse.c calls it through pg_get_viewdef_internal() but don't understand how is it different from e.g., select pg_get_viewdef(...), and that last one is not affected. Thoughts? -- Alex
