Re: multivariate statistics (v19)

From: "Ideriha, Takeshi" <ideriha(dot)takeshi(at)jp(dot)fujitsu(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Robert Haas <robertmhaas(at)gmail(dot)com>, Tatsuo Ishii <ishii(at)postgresql(dot)org>, David Steele <david(at)pgmasters(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: multivariate statistics (v19)
Date: 2017-01-26 09:03:10
Message-ID: 4E72940DA2BF16479384A86D54D0988A565822A9@G01JPEXMBKW04
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

When you have time, could you rebase the pathes?
Some patches cannot be applied to the current HEAD.
0001 patch can be applied but the following 0002 patch cannot be.

I've just started reading your patch (mainly docs and README, not yet source code.)

Though these are minor things, I've found some typos or mistakes in the document and README.

>+ statistics on the table. The statistics will be created in the in the
>+ current database. The statistics will be owned by the user issuing

Regarding line 629 at 0002-PATCH-shared-infrastructure-and-ndistinct-coeffi-v22.patch,
there is a double "in the".

>+ knowledge of a value in the first column is sufficient for detemining the
>+ value in the other column. Then functional dependencies are built on those

Regarding line 701 at 0002-PATCH,
"determining" is mistakenly spelled "detemining".

>@@ -0,0 +1,98 @@
>+Multivariate statististics
>+==========================

Regarding line 2415 at 0002-PATCH, "statististics" should be statistics

>+ <refnamediv>
>+ <refname>CREATE STATISTICS</refname>
>+ <refpurpose>define a new statistics</refpurpose>
>+ </refnamediv>

>+ <refnamediv>
>+ <refname>DROP STATISTICS</refname>
>+ <refpurpose>remove a statistics</refpurpose>
>+ </refnamediv>

Regarding line 612 and 771 at 0002-PATCH,
I assume saying "multiple statistics" explicitly is easier to understand to users
since these commands don't for the statistics we already have in the pg_statistics in my understanding.

>+ [1] http://en.wikipedia.org/wiki/Database_normalization

Regarding line 386 at 0003-PATCH, is it better to change this link to this one:
https://en.wikipedia.org/wiki/Functional_dependency ?
README.dependencies cites directly above link.

Though I pointed out these typoes and so on,
I believe these feedback are less priority compared to the source code itself.

So please work on my feedback if you have time.

regards,
Ideriha Takeshi

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vinayak 2017-01-26 09:04:10 Re: Transactions involving multiple postgres foreign servers
Previous Message Fabien COELHO 2017-01-26 08:09:15 Re: pgbench more operators & functions