Okay, I've applied this piece for now. Not sure I'll have much room
to look at the rest.
Thank you very much!
Rest of patches, rebased.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru
of patches. That makes for easier reviews.
Thanks, will try
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
;hashvalue)
+ goto rerun;
return (void *) ELEMENTKEY(curElem);
}
But for me it looks weird and adds some checks which will takes some CPU time.
03-att_with_hash_value.v5.patch - adds usage of previous patch.
--
Teodor Sigaev
Got it. Here is an updated patch where I added a corresponding comment.
Thank you!
Playing around I found one more place which could easily modified with
hash_seq_init_with_hash_value() call.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
in TypeCacheTypCallback():
I've not read this patch, but IIRC in some places we have a convention
that hash value zero is passed for an sinval reset event (that is,
"flush all cache entries").
regards, tom lane
--
Teodor Sigaev E
I would like to tweak the patch a little bit - change some comments,
add some Asserts, etc. Don't you mind?
You are welcome!
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
```
Try increase -F option of patch.
Anyway, union of both patches in attachment
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/diff --git a/src/backend/utils/cache/typcache.c b/src/backend
total
3) hash_seq_init_with_hash_value.patch
Time: 24975,886 ms (00:24,976)
psql < custom_types_and_array.sql 1,33s user 1,25s system 3% cpu 1:19,77 total
4) both
Time: 89,446 ms
psql < custom_types_and_array.sql 0,72s user 0,52s system 10% cpu 12,137 total
-
will propose pluggable toaster patch a bit later
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW:
http://www.sigaev.ru/
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http
one) which accepts several arguments, one
of which is table AM oid. If that method returns false then toaster
isn't useful with current TAM, storage or/and compression kinds, etc.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http
ke
an API interface to compression to make it configurable. Right now,
module developer could not implement a module with new compression
method and it is a disadvantage.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
because default toaster
stores chunks in heap table.
Thank you!
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
st)
Right
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
les behind:
Oleg Bartunov
Nikita Gluhov
Nikita Malakhov
Teodor Sigaev
[1]
https://www.postgresql.org/message-id/flat/de83407a-ae3d-a8e1-a788-920eb334f...@sigaev.ru
<https://www.postgresql.org/message-id/flat/de83407a-ae3d-a8e1-a788-920eb334f...@sigaev.ru>
--
Teodor Sigaev
will propose pluggable toaster patch a bit later
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
create_table_storage-v1.patch.gz
Description: application/gzip
ke the logic for
resetting the flag, please elaborate on the issues you foresee and one
of the alternative approaches can be tried.
I've attached the updated patch.
Regards,
Greg Nancarrow
Fujitsu Australia
--
Teodor Sigaev E-mail: teo...@sigaev.ru
postmaster.c:4189
#18 0x009a2f63 in ServerLoop () at postmaster.c:1727
#19 0x009a0a0a in PostmasterMain (argc=3, argv=0x7fffe3c8) at
postmaster.c:1400
#20 0x0088deef in main (argc=3, argv=0x7fffe3c8) at main.c:210
--
Teodor Sigaev
concurrent split could be deteced here and it was missed long ago. But
this patch seems a good chance to change this comment.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
It would be easier to figure this out if the btree_gist code weren't
so desperately undocumented. Teodor, do you remember why it's like
this?
Will look.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http
ms", Thomas H. Cormen, Charles E.
Leiserson, Ronald L. Rivest, ISBN 0-07-013143-0
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
diff --git a/src/backend/optimizer/path/costsize.c b/
and if cost_sort() estimates cost is less than 80% (arbitrary
chosen) cost of user-suggested pathkeys then it use our else user pathkeys.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Peter Geoghegan wrote:
On Thu, Jun 28, 2018 at 9:47 AM, Teodor Sigaev wrote:
Current estimation of sort cost has following issues:
- it doesn't differ one and many columns sort
- it doesn't pay attention to comparison function cost and column width
- it doesn't try to count number
://commitfest.postgresql.org/18/1706/
I tried to attack the cost_sort() issues and hope on that basis we can solve
problems with 0002 patch and improve incremental sort patch.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
to change that.
- Empiric constants. I know, it's impossible to remove them at all, but, may
be, we need to find better estimation of them.
[1] https://commitfest.postgresql.org/18/1124/
[2] https://commitfest.postgresql.org/18/1651/
--
Teodor Sigaev E-
into situations where the standy starts to lag behind very
significantly.
+1, we faced with that too
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
node.
Incremental sort patch will improve cases where there is partial match of order.
BTW I get compiler warnings that n_preordered_groups may be used
uninitialized in add_paths_to_grouping_rel, because it's only set in an
if/else branch, and then used further down.
Fixed, but I believe tha
) provided by incremental sort patch
and, it's a pity, it doesn't solve our problem with the impact of the cost of
per-column comparison function and number of its calls.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
BY. In which
case we don't add any Sort, of course.
I hope so
I'm still opposed to adding arbitrary handicap to prioritize the order specified
by user, for the reasons I explained before. We should aim to make the
heuristics/costing reliable enough to make this unnecessary.
+1
--
Teodor
cute query.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
ch follows that if I didn't a mistake.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
le/disable these options during development, because
that makes experiments much easier. But then remove them before commit.
Will do
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
olumn with 100k
distinct values, with actual uniform distribution. I'm pretty sure it'd be more
efficient to place the 100k column first.
Interesting. Will think, thank you
--
Teodor Sigaev E-mail: teo...@sigaev.ru
Teodor Sigaev wrote:
Ah, I think this is the missing, essential component:
CREATE INDEX ON t(right(i::text,1)) WHERE i::text LIKE '%1';
Finally, I reproduce it with attached script.
In attachment simplified version of script. psql uses ordinary sql query
to get info about index with usual
I use FileStly plugin to vim [1]. But I slightly modify it, see in attachment.
FileStyle, sorry.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
; ||
expand('%:e') == "pm" || expand('%:e') == "y" || expand('%:e') == "l"
else
let g:filestyle_plugin = 1
endif
[1] https://www.vim.org/scripts/script.php?script_id=5065
--
Teodor Sigaev E-mail: teo...@sigaev.ru
this reordering optimization.
The first part (additional indexes) seems quite fairly safe, likely to get
committable soon. The other part (ndistinct reordering) IMHO requires more
thought regarding costing and interaction with other query parts.
Thank you for review!
--
Teodor Sigaev
led for relation 1033478
ALTER TABLE
ERROR: cache lookup failed for relation 1034073
ALTER TABLE
ERROR: cache lookup failed for relation 1034650
ALTER TABLE
ERROR: cache lookup failed for relation 1035238
ALTER TABLE
ERROR: cache lookup failed for relation 1035837
will investigate
--
Teodor Sig
Is that considered an actionable problem?
I think so. but I'm not able to reproduce that, I wrote a script to simplify but
it doesn't reproduce too.
And how long to wait to reproduce? I waited for one hour
--
Teodor Sigaev E-mail: teo...@sigaev.ru
Etsuro Fujita
Peter Geoghegan
Amit Kapila
Alexander Korotkov
Thomas Munro
Michael Paquier
Tomas Vondra
Congratulations to all!
+7!
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
I think it will always be set to BTREE_VERSION (See _bt_restore_meta).
You are right, pushed. Thank you!
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Cosmetics change: remove find_sort_group_clause_by_sortref() function added in
v5 patch version because it duplicates existsing get_sortgroupref_clause().
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http
The metapage upgrade should be performed under critical section.
Agree. But after close look I found that btm_version change isn't wal-logged
(near line 2251 in _bt_newroot). So btm_version is not propagated to
replica/backup/etc.
I believe it should be fixed.
--
Teodor Sigaev
yet.
4) Algorithms to reorder columns is proportional to N^2 where N is number of
columns, but I hope it isn't a problem because number of GROUP BY columns isn't
very big usually.
--
Teodor Sigaev E-mail: teo...@sigaev
Thank you. Seems, it works, at least I can't find a counter-example for that.
Tom Lane wrote:
Teodor Sigaev <teo...@sigaev.ru> writes:
I'm not understand why postgres prefers to sort table instead of using
index only scan when query is a simple inner join on composite type.
Query with eq
(cost=0.00..1834.00 rows=10 width=37)
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
test.sql
Description: application/sql
text search over json(b) columns.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
/JSONB values (Dmitry Dolgov)
to
Add function json(b)_to_tsvector to create usable vectors to search in json(b)
(Dmitry Dolgov)
or somehow else. Your wording is about query but actually that functions are
about how to create tsvector from json(b)
Thank you!
--
Teodor Sigaev
thanks to everyone, pushed
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
uot;);
where the subroutine contains the whole ereport() call, and its lookup
table entries are e.g.
gettext_noop("cannot cast jsonb string to type %s")
Thanks for your idea, patch is attached
--
Teodor Sigaev E
How about "cannot cast jsonb $json_type to $sql_type" where $json_type
is the type inside the jsonb (e.g. string, number, boolean, array,
object)?
Yes, that sounds pretty good.
Does anybody have an objections to patch?
--
Teodor Sigaev E
truncations, while preserving the
generic BTreeTupleGetNAtts() assertions.
This isn't a correctness issue, and the extra overhead of unneeded
truncation should be negligible, but what we have now seems confusing
to me.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
Thanks to everyone, v3 is pushed.
Teodor Sigaev wrote:
I don't very happy with rootBuffer added everywhere. ginInsertItemPointers()
and ginPrepareDataScan() now take both args, rootBlkno and rootBuffer,
second could be invalid. As I can see, you did it to call
CheckForSerializableConflictIn
Postgres Company
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
and CheckForSerializableConflictIn() should be added to
ginFindLeafPage() with searchMode = false.
Implemented, v3 is attached.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
commit
to
ginFindLeafPage() with searchMode = false.
Rebased patch is attached.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
commit 42f73743d9ddf576d2dd9ece3979b407cd70cbfe
Author: Teodor Sigaev <
Teodor, are you caught up to the point where it'd be okay to run pgindent,
or are there still patches waiting?
There is a gin predicate lock patch from Heikki, I will work on it. But
I have not objection to run pgindent, I believe gin patch will be easy
to change.
--
Teodor Sigaev
It looks like you pushed v1, which didn't have the tests and other
changes you asked for. Attached patch adds those back.
Oops, I missed, I don't know how... Thank you very much for your quick eye!
--
Teodor Sigaev E-mail: teo...@sigaev.ru
know a correct way to do it :)
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Thank you, pushed.
Peter Geoghegan wrote:
On Tue, Apr 24, 2018 at 9:06 AM, Teodor Sigaev <teo...@sigaev.ru> wrote:
Perfect!
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
to use palloc0 (or memset(0)) for BtreeCheckState.
Now several fields of that structure could be not inited.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
a bit. [1] indicates there's currently no coverage of this function at
all.
[1] https://coverage.postgresql.org/src/backend/nodes/bitmapset.c.gcov.html
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http
leaf page's high key may point to an ancestor
page (at all other times, the leaf level high key's link is not used).
See the nbtree README for full details."
Changed as you suggested.
--
Teodor Sigaev E-mail: teo...
deletion. This removes last naked usage of
ItemPointer(SetInvalid/IsInvalid/GetBlockNumberNoCheck) and uses self-described
macroses. Patch is attached.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http
, which makes
a bit more sense.
Hm, it seems to me, that 350ms is short enough to place it in both core
and amcheck test. I think, basic functionality should be covered by core
tests as we test insert/create.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
-engineering
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
I also think that we could have better conventional regression test
coverage here.
I tried to minimize Michael's test case and add it to patch.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
diff --git a/src
regression test
coverage here.
Will try to invent not so large test.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
decisions are follows, I didn't look at that.
Trivial and naive fix is attached, but for me it looks a bit annoing that we
store pointer (leafhikey) somewhere inside unlocked page.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
Thank you, pushed
Peter Geoghegan wrote:
On Wed, Apr 18, 2018 at 10:47 PM, Teodor Sigaev <teo...@sigaev.ru> wrote:
Thank you, pushed.
Thanks.
I saw another preexisting issue, this time one that has been around
since 2007. Commit bc292937 forgot to remove a comment above
_bt_inse
:
+ /*
+*Cannot leak memory here, TupleDescCopy() doesn't allocate any
+* inner structure, so, plain pfree() should clean all allocated memory
+*/
fixed
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
he culprit (8224de4).
Thanks,
--
Michael
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
was unclear. Attached patch contains
all changes suggested in my previous email.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index
If there are large refactoring or bug-fix patches that haven't landed
yet, then it'd be appropriate to wait for those to get in, but I'm not
aware of such at the moment.
Pls, wait
https://www.postgresql.org/message-id/9c63951d-7696-ecbb-b832-70db7ed3f39b%40sigaev.ru
Thank you.
--
Teodor
code
doesn't just confuse things (e.g. see recent commit 2a67d644). It also
seems like it could actually be harmful. This is code that could only
ever corrupt your database.
I'm fine if Teodor wants to commit that change separately, of course.
--
Teodor Sigaev
nvironment set to 8192.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
der
USE_ASSERT_CHECKING to prevent performance degradation. Users shouldn't
pay for unused feature.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
pushed. Hope, second try will be successful.
Teodor Sigaev wrote:
Thank you, pushed
Amit Langote wrote:
Hi.
On 2018/04/11 0:36, Teodor Sigaev wrote:
Does the attached fix look correct? Haven't checked the fix with
ATTACH
PARTITION though.
Attached patch seems to fix the problem
to use including
column as key column
4) per Peter Geoghegan idea add a error message if somebody adds options to
include column instead silently ignore it.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http
-P0gDCtXbVDkBBG_WcXwCBK=gho4fewu...@mail.gmail.com
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
columns.
Otherwise the collation returned by an index-only scan would be
different from a table scan, no?
+1, dangerous
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Peter Geoghegan idea add a error message if somebody adds options
to include column instead silently ignore it.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
diff --git a/src/backend/catalog/index.c b/src/backend/c
, not a ii_NumIndexKeyAttrs number as
easy to think.
I suggest rename ii_KeyAttrNumbers to ii_AttrNumbers or
ii_IndexAttrNumbers. Opinions?
for (i = 0; i < info1->ii_NumIndexAttrs; i++)
{
if (maplen < info2->ii_KeyAttrNumbers[i])
--
Teodor Sigaev
more work:
for (i = 0; i < info1->ii_NumIndexAttrs; i++)
{
if (maplen < info2->ii_KeyAttrNumbers[i])
Seems, we can go out from ii_KeyAttrNumbers array.
Amit Langote wrote:
Hi.
On 2018/04/11 0:36, Teodor Sigaev wrote:
Does the attached fix look correct? Hav
.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Thank you, pushed
Amit Langote wrote:
Hi.
On 2018/04/11 0:36, Teodor Sigaev wrote:
Does the attached fix look correct? Haven't checked the fix with
ATTACH
PARTITION though.
Attached patch seems to fix the problem. However, I would rather get
rid of modifying stmt->indexPar
for you opinion?
I do not have the time to write a patch right away, but I should be
able to post one in a few days. I want to avoid sending several small
patches.
no problem, we can wait
--
Teodor Sigaev E-mail: teo...@sigaev.ru
ing indexes on
partitioned tables. See the attached patch.
Seems right way, do not modify incoming object and do not copy rather large and
deep nested structure as suggested by Amit.
But it will be better to have a ATTACH PARTITION test too.
--
Teodor Sigaev E-m
;ID" >= 2068113880) AND ("ID" <= 2068629726))
(2 rows)
Before starting investigation of the problem, I will like to know
opinion and may be some advise of people familiar with optimizer:
how difficult will be to handle this case and where to look.
Thanks in advance,
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
not change metapage.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
<noriyoshi.shin...@hpe.com>
*Cc:* PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Teodor Sigaev
<teo...@sigaev.ru>; Peter Geoghegan <p...@bowt.ie>; Jeff Janes
<jeff.ja...@gmail.com>; Anastasia Lubennikova <a.lubennik...@postgrespro.ru>
*Subject:* Re:
the attached patch look sane to you?
I like an idea use metapage locking, thank you. Patch seems good, will you push
it?
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Ugh, I miss your last email where you another locking protocol. Reading.
Teodor Sigaev wrote:
Attached is a test case that demonstrates a case where we miss a serialization
failure, when fastupdate is turned on concurrently. It works on v10, but fails
to throw a serialization error on v11
place for that in
ginoptions function, but ginoptions doesn't has an access to relation structure
and I don't see a reason why it should.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Thank you, pushed.
Peter Geoghegan wrote:
On Sun, Apr 8, 2018 at 11:18 AM, Teodor Sigaev <teo...@sigaev.ru> wrote:
Thank you, fixed
I suggest that we remove some unneeded amcheck tests, as in the
attached patch. They don't seem to add anything.
--
Teodor Sigaev
'indnatts' [-Wunused-variable]
int indnatts = tupleDescriptor->natts;
Thank you, fixed
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Opinions?
Second point, pls. I'd like to see some stage done
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Thank you, fixed
Jeff Janes wrote:
On Sat, Apr 7, 2018 at 4:02 PM, Teodor Sigaev <teo...@sigaev.ru
<mailto:teo...@sigaev.ru>> wrote:
Thanks to everyone, pushed.
Indeed thanks, this will be a nice feature.
It is giving me a compiler warning on non-cassert builds using gcc
(
missed.
I can look at fixing these for you if you like?
If you will do that I will be very grateful
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
Thank you, I looked to buildfarm and completely forget about commitfest site
Andres Freund wrote:
On 2018-04-07 23:02:08 +0300, Teodor Sigaev wrote:
Thanks to everyone, pushed.
Marked CF entry as committed.
Greetings,
Andres Freund
--
Teodor Sigaev E-mail: teo
I'll keep an eye on the buildfarm, since it's late in Russia.
Thank you very much! Now 23:10 MSK and I'll be able to follow during
approximately hour.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
1 - 100 of 165 matches
Mail list logo