Code review comment for lp:~wgrant/launchpad/validpersoncache-no-emailaddress-account

Revision history for this message
Stuart Bishop (stub) wrote :

> +CREATE OR REPLACE VIEW validpersoncache AS (
> +    SELECT emailaddress.person AS id
> +    FROM emailaddress, person, account
> +    WHERE
> +        emailaddress.person = person.id
> +        AND person.account = account.id
> +        AND emailaddress.person IS NOT NULL
> +        AND emailaddress.status = 4
> +        AND account.status = 20
> +);

The emailaddress.person IS NOT NULL clause is redundant, as the first
clause means it will never be NULL.

It is interesting that we don't currently check that person.merged IS
NULL here, although we do in the validpersonorteamcache view. We might
have assumed it was redundant as merged person records will never have
an account? In which case it would have guarded against bad data for
no real cost. I guess we should leave it as it is as I think this view
is only used by legacy code.

> +CREATE OR REPLACE VIEW validpersonorteamcache AS (
> +    SELECT person.id
> +    FROM
> +        person
> +        LEFT JOIN emailaddress ON person.id = emailaddress.person
> +        LEFT JOIN account ON person.account = account.id
> +    WHERE
> +        person.teamowner IS NOT NULL
> +        AND person.merged IS NULL
> +        OR person.teamowner IS NULL
> +        AND account.status = 20
> +        AND emailaddress.status = 4
> +);

I like brackets in mixed AND and OR statements so I don't need to
worry about precedence, rules :)

I just looked at using UNION ALL for the second view, but the
estimates are slightly slower.

« Back to merge proposal