search

2ndQuadrant / audit-trigger

Simple, easily customised trigger-based auditing for PostgreSQL (Postgres). See also pgaudit.
Other
659 stars 241 forks source link

Just converted from hstore to jsonb #32

Closed michelmilezzi closed 1 year ago

michelmilezzi commented 6 years ago
jackhamburger commented 6 years ago

Is actively managing this project?

jackhamburger commented 6 years ago

@michelmilezzi Did you run into an issue with PG10 that hstore broke? Or is this removing a unneeded dependency made obsolete by jsonb?

michelmilezzi commented 6 years ago

@jackhamburger Just removing an unneeded dependency. There was a long time since last activity here, I guess the folks from 2ndQuadrant discontinued this project...

ringerc commented 5 years ago

Moving to jsonb is wise. I'd like to find the time to test and merge this.

It'd be a huge help if you could prepare a test script we could replay to test the extension. One that:

A simple way to do this would be to add a PGXS Makefile and a sql/ and expected/ directory so you could make check, treating it as a PostgreSQL extension. Bonus points for a control file etc.

I will not have time to do this. I can't really merge the jsonb change without some kind of test script but I can't find the time to write one either, so I'm going to have to ask if anyone else @ here is willing.

As for maintenance. This is not and never was a maintained product. It was my personal script I published for others, and re-hosted onto 2ndQ's account later. Like anything else you can contact info@2ndquadrant.com if you're interested in special arrangements, consulting etc. But really this is here as a service to the community because it's better here than on a wiki.

michelmilezzi commented 5 years ago

Ok @ringerc, I'll do that. Thanks for the feedback.

phryneas commented 5 years ago

Hey @michelmilezzi, are you still on this? Would be great to see this merged :)

michelmilezzi commented 5 years ago

@phryneas yes, I do. I'll finish it asap, thanks for the interest :)

sta-szek commented 5 years ago
  • Postgres 10+.

@michelmilezzi hi. You said postgres 10+, why?

I can see jsonb is also in Postgres 9.6 -- which I am using (cannot upgrade). I wanted to switch to jsonb to improve my performance.

Is there anything I need to know about it? Thanks

sta-szek commented 5 years ago

I tried that on my local 9.6 (hstore works) and it throws exception:

Error inserting mock data: SQL Error [42883]: Batch entry 0 INSERT INTO ... VALUES ... was aborted: ERROR: operator does not exist: jsonb - text[]
Hint: No operator matches the given name and argument type(s). You might need to add explicit type casts.
Where: PL/pgSQL function cdc.change_data_capture() line 54 at assignment  Call getNextException to see other errors in the batch.

i am not 100% sure, but i guess it complains about the line cdc_row.row_data = row_to_json(NEW)::JSONB - excluded_cols;

sta-szek commented 5 years ago

I made it working by removing - excluded_cols; from all the places as I don't need this feature.

After testing it turned out that JSONB (at least in 9.6) is ~ 2 times slower than hstore.

10,000 records in table with 69 fields + 7 indices: query: update activity set "timestamp"=CURRENT_TIMESTAMP;

  1. no triggers: ~800ms
  2. hstore: ~2,5s
  3. jsonb: ~4,5s

Just FYI

DavidBoone commented 4 years ago

I also noticed the performance hit when switching from hstore to jsonb. I was able to recover most of that performance by using hstore to calculated changed_fields, then converting that result to jsonb. You can find that modification at https://github.com/michelmilezzi/audit-trigger/pull/1 or https://github.com/2ndQuadrant/audit-trigger/compare/master...DavidBoone:master

trnl commented 4 years ago

@sta-szek, @DavidBoone, @ringerc, are there any chance it will be merged?

iloveitaly commented 1 year ago

@2ndquadrant-ci any chance we could get this merged?

ringerc commented 1 year ago

No, I won't merge this PR, as it incompatibly changes the existing definition. Also, last I checked hstore remained considerably more efficient than jsonb, but that was a while ago, maybe it has improved.

I would be happy to accept a PR that adds a json-flavoured extension with the same functionality, but you might as well just host your own and ask me to link to that instead of rewriting this one in that case.

This was really meant to be a demo / proof of concept for people to adopt and adapt to their needs more than it was a canned ready-to-go project for all needs.