Add a floor of zero to transaction report counts (#2074)

See b/290228682, there are edge cases in which the net_renew would be negative when
a domain is cancelled by superusers during renew grace period. The correct thing
to do is attribute the cancellation to the owning registrar, but that would require
changing the owing registrar of the the corresponding cancellation DomainHistory,
which has cascading effects that we don't want to deal with. As such we simply
floor the number here to zero to prevent any negative value from appearing, which
should have negligible impact as the edge cage happens very rarely, more specifically
when a cancellation happens during grace period by a registrar other than the the
owning one. All the numbers here should be positive to pass ICANN validation.
This commit is contained in:
Lai Jiang 2023-07-12 12:56:09 -04:00 committed by GitHub
parent c24177e8a6
commit 3ea31d024e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 21 additions and 3 deletions

View file

@ -47,7 +47,16 @@ FROM (
END AS clientId,
tld,
report_field AS field,
report_amount AS amount,
-- See b/290228682, there are edge cases in which the net_renew would be negative when
-- a domain is cancelled by superusers during renew grace period. The correct thing
-- to do is attribute the cancellation to the owning registrar, but that would require
-- changing the owing registrar of the the corresponding cancellation DomainHistory,
-- which has cascading effects that we don't want to deal with. As such we simply
-- floor the number here to zero to prevent any negative value from appearing, which
-- should have negligible impact as the edge cage happens very rarely, more specifically
-- when a cancellation happens during grace period by a registrar other than the the
-- owning one. All the numbers here should be positive to pass ICANN validation.
MAX(report_amount, 0) AS amount,
reporting_time AS reportingTime
FROM EXTERNAL_QUERY("projects/%PROJECT_ID%/locations/us/connections/%PROJECT_ID%-sql",
''' SELECT history_type, history_other_registrar_id, history_registrar_id, domain_repo_id, history_revision_id FROM "DomainHistory";''') AS dh

View file

@ -45,7 +45,7 @@ class ActivityReportingQueryBuilderTest {
}
@Test
void testIntermediaryQueryMatch_cloud_sql() {
void testIntermediaryQueryMatch_cloudSql() {
ImmutableList<String> expectedQueryNames =
ImmutableList.of(
ActivityReportingQueryBuilder.REGISTRAR_OPERATING_STATUS,

View file

@ -47,7 +47,16 @@ FROM (
END AS clientId,
tld,
report_field AS field,
report_amount AS amount,
-- See b/290228682, there are edge cases in which the net_renew would be negative when
-- a domain is cancelled by superusers during renew grace period. The correct thing
-- to do is attribute the cancellation to the owning registrar, but that would require
-- changing the owing registrar of the the corresponding cancellation DomainHistory,
-- which has cascading effects that we don't want to deal with. As such we simply
-- floor the number here to zero to prevent any negative value from appearing, which
-- should have negligible impact as the edge cage happens very rarely, more specifically
-- when a cancellation happens during grace period by a registrar other than the the
-- owning one. All the numbers here should be positive to pass ICANN validation.
MAX(report_amount, 0) AS amount,
reporting_time AS reportingTime
FROM EXTERNAL_QUERY("projects/domain-registry-alpha/locations/us/connections/domain-registry-alpha-sql",
''' SELECT history_type, history_other_registrar_id, history_registrar_id, domain_repo_id, history_revision_id FROM "DomainHistory";''') AS dh