commit 87f5baf8e9dfa690fe902bd9c525610da8251e77
parent c3c603a69758d68a387adfd0d2833ee7312e0944
Author: Christian Grothoff <christian@grothoff.org>
Date: Wed, 5 Aug 2026 23:35:43 +0200
log errors if statistic slugs and registerd slugs disagree and fix disagreement...
Diffstat:
5 files changed, 201 insertions(+), 28 deletions(-)
diff --git a/src/backenddb/pg_statistics_helpers.sql b/src/backenddb/pg_statistics_helpers.sql
@@ -376,6 +376,23 @@ CREATE OR REPLACE PROCEDURE merchant_do_bump_number_stat(
LANGUAGE plpgsql
AS $$
BEGIN
+ -- Complain loudly about statistics that nobody ever registered: both
+ -- subroutines below silently do nothing for an unknown slug, so a typo
+ -- in a trigger would otherwise never be noticed.
+ PERFORM
+ FROM (SELECT 1
+ FROM merchant_statistic_bucket_meta
+ WHERE slug=in_slug
+ AND stype='number'
+ UNION ALL
+ SELECT 1
+ FROM merchant_statistic_interval_meta
+ WHERE slug=in_slug
+ AND stype='number') AS known;
+ IF NOT FOUND
+ THEN
+ RAISE WARNING 'Numeric statistic "%" is not registered, event dropped', in_slug;
+ END IF;
CALL merchant_do_bump_number_bucket_stat (in_slug, in_timestamp, in_delta);
CALL merchant_do_bump_number_interval_stat (in_slug, in_timestamp, in_delta);
END $$;
@@ -392,6 +409,23 @@ CREATE OR REPLACE PROCEDURE merchant_do_bump_amount_stat(
LANGUAGE plpgsql
AS $$
BEGIN
+ -- Complain loudly about statistics that nobody ever registered: both
+ -- subroutines below silently do nothing for an unknown slug, so a typo
+ -- in a trigger would otherwise never be noticed.
+ PERFORM
+ FROM (SELECT 1
+ FROM merchant_statistic_bucket_meta
+ WHERE slug=in_slug
+ AND stype='amount'
+ UNION ALL
+ SELECT 1
+ FROM merchant_statistic_interval_meta
+ WHERE slug=in_slug
+ AND stype='amount') AS known;
+ IF NOT FOUND
+ THEN
+ RAISE WARNING 'Amount statistic "%" is not registered, event dropped', in_slug;
+ END IF;
CALL merchant_do_bump_amount_bucket_stat (in_slug, in_timestamp, in_delta);
CALL merchant_do_bump_amount_interval_stat (in_slug, in_timestamp, in_delta);
END $$;
diff --git a/src/backenddb/pg_triggers.sql b/src/backenddb/pg_triggers.sql
@@ -107,7 +107,7 @@ LANGUAGE plpgsql
AS $$
BEGIN
CALL merchant_do_bump_amount_stat
- ('wire-fees-paid'
+ ('total-wire-fees-paid'
,CURRENT_TIMESTAMP(0)::TIMESTAMP
,NEW.wire_fee);
RETURN NEW;
@@ -199,7 +199,7 @@ BEGIN
IF NEW.wire_fee IS NOT NULL
THEN
CALL merchant_do_bump_amount_stat
- ('wire-fees-paid'
+ ('total-wire-fees-paid'
,CURRENT_TIMESTAMP(0)::TIMESTAMP
,NEW.wire_fee);
END IF;
@@ -215,7 +215,7 @@ BEGIN
IF NEW.wire_fee IS NOT NULL AND OLD.wire_fee IS NULL
THEN
CALL merchant_do_bump_amount_stat
- ('wire-fees-paid'
+ ('total-wire-fees-paid'
,CURRENT_TIMESTAMP(0)::TIMESTAMP
,NEW.wire_fee);
END IF;
diff --git a/src/backenddb/sql-schema/merchant-0043.sql b/src/backenddb/sql-schema/merchant-0043.sql
@@ -26,15 +26,18 @@ CREATE PROCEDURE merchant.merchant_0043_init(s TEXT)
LANGUAGE plpgsql
AS $OUTER$
BEGIN
+
+ EXECUTE format('SET LOCAL search_path TO %I', s);
+
-- Add primary constraint, delete constraint-violating values first
- EXECUTE format('DELETE FROM %I.merchant_product_categories f' ||
- ' USING %I.merchant_product_categories d' ||
- ' WHERE f.product_serial = d.product_serial' ||
- ' AND f.category_serial = d.category_serial' ||
- ' AND f.ctid > d.ctid;', s, s);
- EXECUTE format('ALTER TABLE %I.merchant_product_categories' ||
- ' ADD PRIMARY KEY(product_serial, category_serial);', s);
+ DELETE FROM merchant_product_categories f
+ USING merchant_product_categories d
+ WHERE f.product_serial = d.product_serial
+ AND f.category_serial = d.category_serial
+ AND f.ctid > d.ctid;
+ ALTER TABLE merchant_product_categories
+ ADD PRIMARY KEY(product_serial, category_serial);
-- Add unique constraint, delete constraint-violating values first.
-- Without this constraint the 'ON CONFLICT' in
@@ -43,16 +46,17 @@ BEGIN
-- AS IDENTITY column, which the INSERT never supplies), so a replayed
-- issuance duplicated the row and re-incremented the 'issued' counter.
- EXECUTE format('DELETE FROM %I.merchant_issued_tokens f' ||
- ' USING %I.merchant_issued_tokens d' ||
- ' WHERE f.token_family_key_serial = d.token_family_key_serial' ||
- ' AND f.h_contract_terms = d.h_contract_terms' ||
- ' AND f.blind_sig = d.blind_sig' ||
- ' AND f.ctid > d.ctid;', s, s);
- EXECUTE format('ALTER TABLE %I.merchant_issued_tokens' ||
- ' ADD CONSTRAINT merchant_issued_tokens_token_key' ||
- ' UNIQUE (token_family_key_serial, h_contract_terms,' ||
- ' blind_sig);', s);
+ DELETE FROM merchant_issued_tokens f
+ USING merchant_issued_tokens d
+ WHERE f.token_family_key_serial = d.token_family_key_serial
+ AND f.h_contract_terms = d.h_contract_terms
+ AND f.blind_sig = d.blind_sig
+ AND f.ctid > d.ctid;
+ ALTER TABLE merchant_issued_tokens
+ ADD CONSTRAINT merchant_issued_tokens_token_key
+ UNIQUE (token_family_key_serial
+ ,h_contract_terms
+ ,blind_sig);
-- A lock is identified by its UUID: re-posting a lock with a different
-- duration or quantity must update the existing lock for the same UUID
@@ -63,15 +67,47 @@ BEGIN
-- written row (that is the quantity the client last asked for); the
-- AFTER DELETE trigger corrects total_locked for the removed rows.
- EXECUTE format('DELETE FROM %I.merchant_inventory_locks f' ||
- ' USING %I.merchant_inventory_locks d' ||
- ' WHERE f.product_serial = d.product_serial' ||
- ' AND f.lock_uuid = d.lock_uuid' ||
- ' AND f.ctid < d.ctid;', s, s);
- EXECUTE format('ALTER TABLE %I.merchant_inventory_locks' ||
- ' ADD CONSTRAINT merchant_inventory_locks_product_lock_key' ||
- ' UNIQUE (product_serial, lock_uuid);', s);
+ DELETE FROM merchant_inventory_locks f
+ USING merchant_inventory_locks d
+ WHERE f.product_serial = d.product_serial
+ AND f.lock_uuid = d.lock_uuid
+ AND f.ctid < d.ctid;
+ ALTER TABLE merchant_inventory_locks
+ ADD CONSTRAINT merchant_inventory_locks_product_lock_key
+ UNIQUE (product_serial, lock_uuid);
+
+ -- merchant_deposits_insert_statistics_trigger() bumps 'deposits-received'
+ -- and 'deposits-fees-paid', and GET /private/statistics-report/transactions
+ -- reads 'deposits-received'. Both slugs were registered by merchant-0028,
+ -- but the default registration for per-instance schemata introduced with
+ -- merchant-0037 was copied from merchant-0014 and hence predates them, so
+ -- no instance created since then collects deposit statistics at all (the
+ -- bump procedures return silently for an unregistered slug).
+ -- Note that 'deposits-fees-paid' overlaps with 'total-deposit-fees-paid':
+ -- the former is collected when the deposit is recorded, the latter when the
+ -- order is marked as paid, so the two do not necessarily agree.
+ INSERT INTO merchant_statistic_bucket_meta
+ (slug
+ ,description
+ ,stype
+ ,ranges
+ ,ages)
+ VALUES
+ ('deposits-received'
+ ,'total amount customers deposited to us (including deposit fees)'
+ ,'amount'
+ ,ARRAY['hour'::merchant.statistic_range, 'day', 'week', 'month', 'quarter', 'year']
+ ,ARRAY[72, 14, 12, 24, 12, 10]
+ ),
+ ('deposits-fees-paid'
+ ,'total amount in deposit fees paid by us or our customers'
+ ,'amount'
+ ,ARRAY['hour'::merchant.statistic_range, 'day', 'week', 'month', 'quarter', 'year']
+ ,ARRAY[72, 14, 12, 24, 12, 10]
+ )
+ ON CONFLICT DO NOTHING;
+ SET LOCAL search_path TO merchant;
END
$OUTER$;
diff --git a/src/backenddb/sql-schema/meson.build b/src/backenddb/sql-schema/meson.build
@@ -125,6 +125,7 @@ generated_sql = [
['merchant-0043.sql'],
['merchant-0044.sql'],
['merchant-0045.sql'],
+ ['merchant-0046.sql'],
]
foreach g : generated_sql
diff --git a/src/backenddb/test_merchantdb.c b/src/backenddb/test_merchantdb.c
@@ -30,6 +30,7 @@
#include "merchantdb_lib.h"
#include "helper.h"
#include "merchant-database/gc.h"
+#include "merchant-database/iterate_statistic_bucket_amounts.h"
#include "merchant-database/iterate_statistic_bucket_counters.h"
#include "merchant-database/iterate_kyc_statuses.h"
#include "merchant-database/insert_kyc_status.h"
@@ -9454,6 +9455,87 @@ test_statistics_bucket_gc (const struct InstanceData *instance)
/**
+ * Counts the amount buckets we are called for.
+ *
+ * @param cls a `struct CountBuckets_Closure *`
+ * @param description description of the statistic
+ * @param bucket_start start time of the bucket
+ * @param bucket_end end time of the bucket
+ * @param bucket_range range of the bucket
+ * @param cumulative_amounts_len length of @a cumulative_amounts
+ * @param cumulative_amounts the cumulative amounts
+ */
+static void
+count_bucket_amount_cb (void *cls,
+ const char *description,
+ struct GNUNET_TIME_Timestamp bucket_start,
+ struct GNUNET_TIME_Timestamp bucket_end,
+ const char *bucket_range,
+ unsigned int cumulative_amounts_len,
+ const struct TALER_Amount cumulative_amounts[static
+ cumulative_amounts_len])
+{
+ struct CountBuckets_Closure *cbc = cls;
+
+ (void) description;
+ (void) bucket_start;
+ (void) bucket_end;
+ (void) bucket_range;
+ (void) cumulative_amounts_len;
+ (void) cumulative_amounts;
+ cbc->count++;
+}
+
+
+/**
+ * Bumps the amount statistic @a slug and checks that the bump was
+ * actually recorded in the bucket statistics. Fails if @a slug is not
+ * registered in @a instance, as the bump procedures silently ignore
+ * statistics nobody registered.
+ *
+ * @param instance the instance to collect statistics for.
+ * @param slug the statistic to bump.
+ * @return 0 on success, 1 otherwise.
+ */
+static int
+test_statistics_amount_slug_collected (const struct InstanceData *instance,
+ const char *slug)
+{
+ struct CountBuckets_Closure cbc = {
+ .count = 0
+ };
+ char *sql;
+
+ TEST_SET_INSTANCE (instance->instance.id,
+ GNUNET_DB_STATUS_SUCCESS_ONE_RESULT);
+ GNUNET_asprintf (&sql,
+ "CALL merchant_do_bump_amount_stat"
+ " ('%s'"
+ " ,CURRENT_TIMESTAMP(0)::TIMESTAMP"
+ " ,ROW(5,0,'EUR')::merchant.taler_amount_currency)",
+ slug);
+ {
+ int ret;
+
+ ret = statistics_exec_sql (sql);
+ GNUNET_free (sql);
+ TEST_RET_ON_FAIL (ret);
+ }
+ TEST_COND_RET_ON_FAIL (0 <=
+ TALER_MERCHANTDB_iterate_statistic_bucket_amounts (
+ pg,
+ instance->instance.id,
+ slug,
+ &count_bucket_amount_cb,
+ &cbc),
+ "Lookup of bucket statistics failed\n");
+ TEST_COND_RET_ON_FAIL (0 < cbc.count,
+ "Statistic bumped by a trigger is not registered\n");
+ return 0;
+}
+
+
+/**
* Prepares for the statistics tests.
*
* @param[out] cls the closure to initialize.
@@ -9490,6 +9572,26 @@ run_test_statistics (struct TestStatistics_Closure *cls)
TEST_RET_ON_FAIL (test_insert_instance (&cls->instance,
GNUNET_DB_STATUS_SUCCESS_ONE_RESULT));
TEST_RET_ON_FAIL (test_statistics_bucket_gc (&cls->instance));
+ /* Every amount statistic bumped by a trigger in pg_triggers.sql must
+ be registered in the per-instance schema. */
+ TEST_RET_ON_FAIL (test_statistics_amount_slug_collected (
+ &cls->instance,
+ "payments-received-after-deposit-fee"));
+ TEST_RET_ON_FAIL (test_statistics_amount_slug_collected (
+ &cls->instance,
+ "total-deposit-fees-paid"));
+ TEST_RET_ON_FAIL (test_statistics_amount_slug_collected (
+ &cls->instance,
+ "total-wire-fees-paid"));
+ TEST_RET_ON_FAIL (test_statistics_amount_slug_collected (
+ &cls->instance,
+ "refunds-granted"));
+ TEST_RET_ON_FAIL (test_statistics_amount_slug_collected (
+ &cls->instance,
+ "deposits-received"));
+ TEST_RET_ON_FAIL (test_statistics_amount_slug_collected (
+ &cls->instance,
+ "deposits-fees-paid"));
return 0;
}
