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
patches. That makes for easier reviews.
Thanks, will try
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
}
+ else if (status->hasHashvalue && status->hashvalue !=
+curElem->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 - ad
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
ue 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
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
system 6% cpu 41,220 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%
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
name, but I don't known better 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
e chosen in toaster's options (not implemented yet) or even make
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/
heap because default toaster
stores chunks in heap table.
Thank you!
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
be expanded without rewriting
everything (to avoid O(N^2) cost)
Right
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
tions, discussions, objections and advices.
Thank you.
Peoples 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-92
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
current scenarios with the
current mechanism in place. If you still dislike 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
at 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 S
rent 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
ding
preordered keys and this form could be directly used in GROUP BY and incremental
sort patches.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
V8 contains fixes of Tomas Vondra complaints:
- correct usage of comparison_cost
- remove uneeded check of sortop
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
diff --git a/src/backend
top);
funcCost = get_func_cost(funcOid);
}
Safety first :). Will remove.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
1 Nov 2017
[5] "Introduction to algorithms", 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 --g
s now 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
olumns now? In what way would that not be a bug?
Because index only scan doesn't support expression index and, hence, expressions
in included columns are not useful. It could be changed in future but, suppose,
not in v11.
--
Teodor Sigaev
Pls, have a look at https://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
me and equal to 1. May be,
it's time 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/
--
asy to get 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/
dy mess with
the order of group clauses - there are ways to get around it (subquery
with OFFSET 0) but it's much less clear.
I like a moment when objections go away :)
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
;s used it
2) if there is a required order - let's match that order to prevent extra sort
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_re
cost_incremental_sort() 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
ORDER 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.
e the optimal way to
execute query.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
ry
to reorder it. Current patch follows that if I didn't a mistake.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
C" that would
allow us to enable/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/
h 1M distinct values,
where a single value represents 99% of the rows. And another column 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
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
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/
;) == "hpp" || expand('%:e') == "pl" ||
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
initial review - I do suggest splitting the patch into two
parts. One that does the index magic, and one for this reordering optimization.
The first part (additional indexes) seems quite fairly safe, likely to get
committable soon. The other part (ndistinct reordering) IMHO requires mor
up failed 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
--
concurrent with the
VACUUM and ALTER, but yours is running consecutively.
both loops run in backgound. I tried to run two scripts - and got a lot of
deadlocks but not a probem reproduction.
--
Teodor Sigaev E-ma
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...
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 S
field if it's not initialized 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
Thank you. Seems, it works, at least I can't find a counter-example for that.
Tom Lane wrote:
Teodor Sigaev 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 equality c
Scan on b (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/
on");
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
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
eeded 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...
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
CheckForSerializableConfl
ssian Postgres Company
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
d 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/
c
e added 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
Date:
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 S
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...
x27;t 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 wrote:
Perfect!
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
s, I suggest 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:
the target 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
uring page
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
amcheck enhancement, 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-ma
seems over-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
so think that we could have better conventional regression test
coverage here.
Will try to invent not so large test.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
eTupleSetNAtts. And some wrong
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
Thank you, pushed
Peter Geoghegan wrote:
On Wed, Apr 18, 2018 at 10:47 PM, Teodor Sigaev 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_insertonpg() (the 'afte
n before commit:
+ /*
+*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/
;t do that in v1, sorry, I 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
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
code within
_bt_insertonpg() is broken code, and not just dead code. The 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 wan
test
with MALLOC_MMAP_THRESHOLD_ environment set to 8192.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
ll under
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 pr
that we don't try 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
rg/message-id/CAJEAwVE0rrr+OBT-P0gDCtXbVDkBBG_WcXwCBK=gho4fewu...@mail.gmail.com
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
ying table 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/
ea 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/catalog/index.c
ind
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? Haven&#
.
--
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->ind
ro).
What is suspicious place 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
covering 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
x Cond: (("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/
cess will not change metapage.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
:* PostgreSQL Hackers ; Teodor Sigaev
; Peter Geoghegan ; Jeff Janes
; Anastasia Lubennikova
*Subject:* Re: WIP: Covering + unique indexes.
Hi!
On Mon, Apr 9, 2018 at 5:07 PM, Shinoda, Noriyoshi <mailto:noriyoshi.shin...@hpe.com>> wrote:
I tested this feature and found a document
n concurrently. Does 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
hat 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 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 E-mail
1 - 100 of 181 matches
Mail list logo
