В письме от 19 октября 2017 14:20:52 Вы написали:
> Yeah, it would perhaps be good idea to ensure we don't break things that
> are documented to work. If the tests don't take too long, I'm not
> opposed to testing every single option. As you say, code coverage is
> important but it's not the
В письме от 19 октября 2017 14:20:52 Вы написали:
> I'm hesitant to hardcode things like the number of bits in bloom, as you
> had in the original. If I understand correctly, that number could
> change with compile options (different blocksize?), so I removed that
> part.
#define
В письме от 3 октября 2017 11:48:43 пользователь Michael Paquier написал:
> > src/backend/access/common/reloptions.c get only 7 lines, it was quite
> > covered by existing test, but all most of the access methods gets some
> > coverage increase:
> >
> > src/backend/access/brin 1268 ->
While working with reloptions refactoring patch, I've written series of tests
that triggers reloptions related code in all access methods. (I needed it to
make sure I did not break anything while coding)
I've included these tests to that patch.
Meanwhile Alvaro suggested to commit these tests
В письме от 3 сентября 2017 11:45:43 пользователь Alvaro Herrera написал:
> > I've just made sure that patch is still applyable to the current master.
> >
> > And I am still waiting for reviews :-)
>
> I see that this patch adds a few tests for reloptions, for instance in
> bloom. I think we
This thread continues discussion at
https://www.postgresql.org/message-id/20170903094543.kkqdbdjuxwa5z6ji@alvherre.pgsql
(Shortly: I refactored reloptions code, Alvaro offered to commit tests before
the full patch)
> I see that this patch adds a few tests for reloptions, for instance in
>
Hi All!
I am about to set "Ready for commit" status to this patch. So there is my
summary for commiter, so one does not need to carefully read all the thread.
This patch is consists of three parts. May be they should be commited
separately, then Fabien will split them, I think.
1. The tests.
В письме от 25 июня 2017 20:42:58 пользователь Fabien COELHO написал:
> > may be this it because of "end_offset + 1" in expr_scanner_chomp_substring
> > ? Why there is + 1 there?
>
> Thanks for the debug! Here is a v9 which does a rebase after the
> reindentation and fixes the bad offset.
Sorry I
В письме от 15 июня 2017 21:10:12 Вы написали:
> > As for me, I would do expr_scanner_chomp_substring(PsqlScanState, int,
> > int&); that changes end_offset as desired...
>
> Why not.
>
> > And use it instead of end_offset = expr_scanner_offset(sstate) - 1;
>
> I removed these?
>
> > The
В письме от 25 июня 2017 19:03:54 пользователь Fabien COELHO написал:
> Hello Nikolay,
>
> >> Is the attached version better to your test?
> >
> > I've expected from expr_scanner_chomp_substring to decrement end_offset,
> > so it would work more like perl chomp function, but the way you've done
В письме от 8 июня 2017 19:56:02 пользователь Fabien COELHO написал:
> > So this should be fixed in both expr_scanner_get_substring cases, and keep
> > last symbol only if it is not "\n".
>
> Indeed, this is a mess. The code assumes all stuff is a line ending with
> '\n', but this is not always
"like more"
> if a native English speaker or someone else has an opinion, it is
> welcome.
Ok, let's leave this for commiter to make final decision.
> Also, if someone could run a test on windows, it would be great.
I'll try to ask a friend of mine to run this on windows...
--
Nikolay Shaplov, independent Perl & C/C++ developer. Available for hire.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
you would call processXactStats in this while loop?
And each time it would do st->cnt++
And this while loop is inside for(;;) in which as I said above we can do st-
>cnt++ not more than once. I see no logic here.
PS This is a fast reply. May be it will make things clear fast wither for me
en somebody else should say "I've
read the code, and I am sure it is good" instead of me. And it would be his
responsibility then. But if you ask me, issues listed above are very
important, and until they are solved I can not tell "the code is good", and I
see no way
>> the script into a file, checking for possible errors when doing so, then
> >> unlinking the file and checking for errors again after the run.
> >
> > I think I was wrong about deleting file after tests are finished. Better
> > keep them.
>
> Hmmm... Then w
k again about maintainability. Having 36 files, most of them <100b
size -- is a very bad idea for maintainability. If each commit of new
functionality would add 36 small files, we will drown in these files quite
soon.
These files should be generated on fly. I am 100% sure of it.
The way they are
hen testing is done.
I think that job for creating and removing these files should be moved to
pgbench and pgbench_likes. But there is more then one way to do it. ;-)
That's what I've noticed so far... I will give this patch more attentive look
soon.
--
Nikolay Shaplov, independent Perl & C/C++ dev
of this option completely, and
thus autosummarize were forever set to false.
And in this obviously broken situation all tests were successful when I run
them. May be there can be a way to make sure it is really works, and one
should add it to the tests?
--
Nikolay Shaplov, independent Perl &
В письме от 26 марта 2017 15:02:12 пользователь Alvaro Herrera написал:
> Nikolay Shaplov wrote:
> > If I would think about it now: we always know how many options we will
> > have. So we can just pass this number to palloc and assert if somebody
> > adds more options then e
is
file contains only the public interface routines." comment at the beginning,
and there is no static functions inside. I do not know what is the right way
to hide hashgetreloptcatalog function here...
What would you advise?
--
Nikolay Shaplov, independent Perl & C/C++ developer. A
В письме от 26 марта 2017 15:02:12 Вы написали:
> Nikolay Shaplov wrote:
> > If I would think about it now: we always know how many options we will
> > have. So we can just pass this number to palloc and assert if somebody
> > adds more options then expected... What do yo t
have something to say about the new
> > functionality that this patch provides.
>
> Thanks Álvaro, I'll look the patch and try to help in some way.
Thank you, that would be great!
--
Nikolay Shaplov, independent Perl & C/C++ developer. Available for hire.
--
Sent vi
but
> visually it's clearly wrong. ... Actually there are whitespace-style
> violations in several places; please fix using pgindent (after adding
> any new typedefs your defining to typedefs.list).
I will run pgindent on my code.
--
Nikolay Shaplov, independent Perl &
; section, because I think user should be able to see full
description of the options in the section where they are described, not in the
introduction.
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/doc/src/sgml/bloom.sgml b/doc/src/sgm
В письме от 31 мая 2016 15:38:38 пользователь Robert Haas написал:
> >>> 99% of the time, you'd be right. But this is an unusual case, for the
> >>> reasons I mentioned before.
> >>
> >> I tend to agree with Nikolay. I can't see much upside in making this
> >> change. At best, nothing will
В письме от 27 мая 2016 15:05:58 Вы написали:
> Nikolay Shaplov wrote:
> > Story start from the point that I found out that a.m. can not forbid
> > changing some of it's reloptions with ALTER INDEX command. That was not
> > necessary before, because all reloptions at that exi
В письме от 24 мая 2016 17:12:16 пользователь Nikolay Shaplov написал:
While working on this patch I met some difficulties that makes me to completely
rewrite a code that is responsible for interacting reloptions.c with access
methods.
Story start from the point that I found out that a.m. can
. It seems logical to me.
// Actually I did not expected any discussion for this case. Documentations
missed an optional keyword, documentation should be fixed. I am surprised that
there can be any other opinions ;-)
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian
it's own data structures.
May be there is cases when you will really need such behavior. But it is quite
different task, have almost nothing in common with things I am going to do :-)
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
--
Sent via
В письме от 24 мая 2016 12:47:20 пользователь Tom Lane написал:
> Nikolay Shaplov <n.shap...@postgrespro.ru> writes:
> > If I read gram.y code for insert statement, I see that there is an
> > optional
> > USING keyword before opclass name
&g
I
would like to remove relopt_kind at all. Because it spoils live in that case.
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
toast, views and so on, should stay in
src/backend/access/common/reloptions.c but should be stored as separate option
catalog for each type. I still not sure what to do with
RELOPT_KIND_HEAP | RELOPT_KIND_TOAST options. But one or another solutions can
be found...
--
Nikolay Shapl
think I will start another thread for
it. I will write another letter for relopt_kind remove issue...
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
index 05dbe87..f439403 100644
--- a/contrib/bl
rk.
Patch in the attachment fixes an example. Please commit it ;-)
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/doc/src/sgml/bloom.sgml b/doc/src/sgml/bloom.sgml
index 7349095..ab84b69 100644
--- a/doc/src/sgml/bloom.sgml
+++ b/doc/src/sgml/
*/ { $$ = NIL; }
;
but it the documentation this keyword is omitted.
I'd like to offer a patch that fixes this problem
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref
uld be specified only once, in .h file.
So I would offer a patch for all access methods. That changes amsupport and
amstrategies from numbers to defined constants. (I've added two of them where
they were missing)
See attachment.
If it is good, I will add it to the commitfest.
--
Nikolay Shap
В письме от 25 ноября 2015 22:27:57 пользователь Michael Paquier написал:
> On Wed, Nov 25, 2015 at 10:16 PM, Nikolay Shaplov <n.shap...@postgrespro.ru>
> wrote:
> > В письме от 18 ноября 2015 15:52:54 пользователь Michael Paquier написал:
> > Teodor Sigaev asked a
В письме от 18 ноября 2015 15:52:54 пользователь Michael Paquier написал:
Teodor Sigaev asked a very good question: does it properly do upgrade from 1.3
to 1.4
I've rechecked and fixed
here is a patch.
> On Wed, Nov 18, 2015 at 3:10 AM, Nikolay Shaplov wrote:
> > Everything seems
В письме от 19 ноября 2015 10:57:27 пользователь Tom Lane написал:
> Nikolay Shaplov <n.shap...@postgrespro.ru> writes:
> > But this is not the issue. We can change it any way we like. The question
> > do we need such example at all, or no?
>
> I'm kind of -1 on the id
В письме от 19 ноября 2015 09:39:41 пользователь Jim Nasby написал:
> On 11/19/15 8:42 AM, Nikolay Shaplov wrote:
> > +sub psql_ok
> > +{
> > + my $db = shift;
> > + my $sql = shift;
> > + my $comment = shift;
>
> Isn't the preferred metho
?
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/src/test/Makefile b/src/test/Makefile
index b713c2c..a45129c 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -17,7 +17,7 @@ SUBDIRS = regress isolation modules
# We don't build or execute
the same file, nobody will guess what the difference
between these function and why are they named in such a strange way.
If it is ok, set proper status, and let commiter do his job :-)
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
diff --git
В письме от 28 октября 2015 16:57:36 пользователь Michael Paquier написал:
> On Sat, Oct 17, 2015 at 1:48 AM, Michael Paquier wrote:
> > On Sat, Oct 17, 2015 at 5:15 AM, Nikolay Shaplov wrote:
> >> Or it's ready to commit, and just not marked this way?
> >
> > No,
Paquier wrote:
> On Thu, Oct 1, 2015 at 8:13 PM, Nikolay Shaplov wrote:
>> В письме от 30 сентября 2015 13:49:00 пользователь Michael Paquier
> написал:
>>> - errmsg("input page too small (%d
&g
This patch changes in
http://www.postgresql.org/docs/9.5/static/sql-createtype.html
"variable length" into "variable-length", since in other places there it is
written with "-" in the middle, not " ".
--
Nikolay Shaplov
Postgres Professional: htt
e the mention to "please notice". Still as this is
> already described in htup_details.h there is no real point to describe it
> again here: that's a bitmap of NULLs.
heap_page_items returns text(!) as t_bits. This is unexpected. This is not
described in page layout documentation. We shou
-
1 | 0100 |
\x010002000300040005000600070008000a00
(1 row)
16 bits as expected.
So I would keep my version of bits_len calculation
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
--
Sent via pgsql-hacke
class, t_data, t_infomask,
> t_infomask2, t_bits) FROM heap_page_items(get_raw_page('pg_class', 0));
> And actually this query crashes.
Oh... It crached because I did not check that t_data can actually be NULL.
There also was a bug in original pageinspect, that did not get t_bits right
whe
s.h (the
> documentation is actually incorrect, I sent a separate patch), and I
> see no point in duplicating such low-level descriptions, that would be
> double maintenance for the same result.
What do you think about this? (I've changed only heap_tuple_items
documentation there)
--
Niko
did not reach it, in my
opinion... So I'd prefer to keep it there.
> Btw, shouldn't the ereport messages in tuple_data_split use
> error_level instead of ERROR?
These errors are in the part where I parse t_bits from text back to bit
representation. Here there is not chance to get non-
cumentation.
Hope it will be the last one. :-)
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Companydiff --git a/contrib/pageinspect/Makefile b/contrib/pageinspect/Makefile
index aec5258..e4bc1af 100644
--- a/contrib/pageinspect/Makefile
+++ b/contrib/pageinspect/Makefile
@@
eave as it is.
I am lazy, and does not feel confidence about touching main documentation, so I
would prefer to leave as it is.
> The example of tuple_data_split in the docs would be more interesting
> if embedded with a call to heap_page_items.
This example would be almost not readable. Ma
optional argument
do_detoast that allows to get bytea[] of detoasted attribute data.
I've decided that there is no real need in warning_mode in
heap_page_item_attrs so there is no such argument there.
So now it is still RFC. Final patch with documentation will come soon
--
Nikolay Shaplov
Postgres
> This is definitely something you want to control with a switch.
Ok.Let's come to the final decision with tuple_data_parse, and i will add this
switch there and to pure sql heap_page_item_attrs
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
--
S
В письме от 8 сентября 2015 11:53:24 Вы написали:
> On Sat, Sep 5, 2015 at 1:05 AM, Nikolay Shaplov wrote:
> > В письме от 4 сентября 2015 14:58:29 пользователь Michael Paquier написал:
> > > Documentation is missing, that would be good to have to understand what
> > >
or commit. So I would know for sure that function definition would
not change, so had not to rewrite it again and again.
So if it is possible I would write documentation and test when this patch is
already approved.
> Code has some whitespaces.
I've found and removed some. Hope that was
В письме от 3 августа 2015 15:35:23 пользователь Alvaro Herrera написал:
> Nikolay Shaplov wrote:
> > This patch adds several new functions, available from SQL queries. All
> > these functions are based on heap_page_items, but accept slightly
> > different arguments and has
that this may be a bug.
And also I'd like reviewers to see proper patch as "Latest patch", if someone
can manually fix that, it would be great!
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
--
Sent via pgsql-hackers mailing list (pgs
В письме от 3 августа 2015 15:35:23 Вы написали:
Nikolay Shaplov wrote:
This patch adds several new functions, available from SQL queries. All
these functions are based on heap_page_items, but accept slightly
different arguments and has one additional column at the result set
В письме от 3 августа 2015 14:30:46 пользователь Michael Paquier написал:
On Mon, Aug 3, 2015 at 1:03 AM, Nikolay Shaplov
n.shap...@postgrespro.ru wrote:
Hi!
I've created a patch for pageinspect that allows to see data stored in the
tuple.
This patch has two main purposes:
1
be it would be better to change it back for better backward compatibility.
Attached patched is in almost ready state. It has some formatting issues.
I'd like to hear HACKER's opinion before finishing it and sending to
commitfest.
--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
61 matches
Mail list logo
