Add an option to ignore queries with conditional expressions

[gajus]

I saw the thread where the decision was made to not support conditional queries (https://github.com/ts-safeql/safeql/issues/100), and I understand the rationale.

However, I am attempting to adopt safeql in a huge codebase, and this is causing a flood of errors.

I think a reasonable solution would be to just add an option to ignore errors / queries that contain conditional expressions.

[karlhorky]

@gajus hey, nice to see you here! 👋

What do you think about the following (this is what we did on a small-medium size codebase, where we also experienced many reported problems):

1. Use manual ESLint disables for now
2. Slowly rewrite the queries to use CASE WHEN ... THEN ... ELSE ... END

I think that adding an option to ignore such errors would have the downside of making it easier to make mistakes in future... 🤔 (not to mention moving further away from a zero-config codebase for SafeQL)

[gajus]

Slowly rewrite the queries to use CASE WHEN ... THEN ... ELSE ... END

That's not something we are planning to do, so it would be nice to have an escape hatch for those who are willing to accept the compromise (of skipping those queries).

Using ESLint disable is an option, but it would mean polluting the codebase, which I am not fan of. We would probably just patch safeql to fit our use case.

For context, I think that conversation is a bit simplistic in terms of how it evaluates the ability to express conditional fragments in raw SQL. Here is just one example of what such a query could look like:

export const resolve = resolverBuilders.Query.adminUserSearch()
  .authenticated()
  .authorized('SEARCH_USERS')
  .resolve(async (_root, args, context, info) => {
    const { after, filter, first, search } = args;
    const { loaders, pool, services } = context;

    let googlePlaceId: null | number;
    if (filter?.locationGooglePlaceId) {
      googlePlaceId = await upsertGooglePlace(
        services.googlePlace,
        pool,
        filter.locationGooglePlaceId,
      );
    }

    const matchOnExactEmail = search && isEmailSyntacticallyValid(search);

    return await loaders.UserSearchResults.load({
      cursor: after,
      info,
      limit: first || 200,
      orderBy: (columns) => {
        const orderBy: Array<[SqlToken, 'ASC' | 'DESC']> = [];
        if (search && !matchOnExactEmail) {
          orderBy.push([
            sql.unsafe`(${columns.name} <-> ${search}) - (${columns.name} ilike ${search} || '%')::integer`,
            'ASC',
          ]);
        }

        if (googlePlaceId) {
          orderBy.push([
            sql.unsafe`
              ST_Distance(
                        (
                          SELECT
                            ST_Centroid(ST_MakeLine(gp1.viewport_southwest, gp1.viewport_northeast))
                          FROM google_place gp1
                          WHERE
                            gp1.id = ${googlePlaceId}
                        ),
                        (
                          SELECT
                          ST_Centroid(ST_MakeLine(gp1.viewport_southwest, gp1.viewport_northeast))
                          FROM google_place gp1
                          WHERE
                            gp1.id = ${columns.googlePlaceId}
                        )
                      )
            `,
            'ASC',
          ]);
        }

        orderBy.push([columns.id, 'ASC']);

        return orderBy;
      },
      where: (columns) => {
        const where = [];

        if (search) {
          if (matchOnExactEmail) {
            where.push(sql.fragment`${columns.emailAddress} ilike ${search}`);
          } else {
            where.push(sql.fragment`(${columns.name} <-> ${search} < 0.9)`);
          }
        }

        if (filter) {
          const {
            hasBeenFeaturedOnSocial,
            hasRating,
            isCommunitySlackMember,
            isSocialFeatureEligible,
            isTopContractor,
            maxAutoRating,
            maxLastRatedAt,
            maxNumberOfPortfolioProjects,
            maxNumberOfProductizedServices,
            maxRating,
            minAutoRating,
            minLastRatedAt,
            minNumberOfPortfolioProjects,
            minNumberOfProductizedServices,
            minRating,
            profileUpdatedSinceBeingRated,
            roleName,
            status,
          } = filter;

          if (typeof hasRating === 'boolean') {
            where.push(sql.fragment`${columns.hasRating} = ${hasRating}`);
          }

          if (typeof isCommunitySlackMember === 'boolean') {
            where.push(
              sql.unsafe`${columns.isCommunitySlackMember} = ${isCommunitySlackMember}`,
            );
          }

          if (typeof isSocialFeatureEligible === 'boolean') {
            where.push(
              sql.unsafe`${columns.isSocialFeatureEligible} = ${isSocialFeatureEligible}`,
            );
          }

          if (typeof hasBeenFeaturedOnSocial === 'boolean') {
            where.push(
              sql.unsafe`${columns.hasBeenFeaturedOnSocial} = ${hasBeenFeaturedOnSocial}`,
            );
          }

          if (typeof isTopContractor === 'boolean') {
            where.push(
              sql.unsafe`${columns.isTopContractor} = ${isTopContractor}`,
            );
          }

          if (typeof minNumberOfPortfolioProjects === 'number') {
            where.push(
              sql.unsafe`${columns.numberOfPortfolioProjects} >= ${minNumberOfPortfolioProjects}`,
            );
          }

          if (typeof maxNumberOfPortfolioProjects === 'number') {
            where.push(
              sql.unsafe`${columns.numberOfPortfolioProjects} <= ${maxNumberOfPortfolioProjects}`,
            );
          }

          if (typeof minNumberOfProductizedServices === 'number') {
            where.push(
              sql.unsafe`${columns.numberOfProductizedServices} >= ${minNumberOfProductizedServices}`,
            );
          }

          if (typeof maxNumberOfProductizedServices === 'number') {
            where.push(
              sql.unsafe`${columns.numberOfProductizedServices} <= ${maxNumberOfProductizedServices}`,
            );
          }

          if (typeof minAutoRating === 'number') {
            where.push(sql.fragment`${columns.autoRating} >= ${minAutoRating}`);
          }

          if (typeof maxAutoRating === 'number') {
            where.push(sql.fragment`${columns.autoRating} <= ${maxAutoRating}`);
          }

          if (typeof minRating === 'number') {
            where.push(sql.fragment`${columns.manualRating} >= ${minRating}`);
          }

          if (typeof maxRating === 'number') {
            where.push(sql.fragment`${columns.manualRating} <= ${maxRating}`);
          }

          if (typeof minLastRatedAt === 'string') {
            where.push(
              sql.fragment`${columns.lastRatedAt} >= ${minLastRatedAt}`,
            );
          }

          if (typeof maxLastRatedAt === 'string') {
            where.push(
              sql.fragment`${columns.lastRatedAt} <= ${maxLastRatedAt}`,
            );
          }

          if (profileUpdatedSinceBeingRated === true) {
            where.push(
              sql.unsafe`${columns.updatedAt} > ${columns.lastRatedAt}`,
            );
          }

          if (status && status.length >= 1) {
            const condition = [];
            if (status.includes('APPROVED')) {
              condition.push(
                sql.unsafe`NOT('{"FLAGGED_USER","BLOCKED_USER"}'::text[] && ${columns.userGroupNids})`,
              );
            }

            if (status.includes('FLAGGED')) {
              condition.push(
                sql.unsafe`'FLAGGED_USER' = ANY(${columns.userGroupNids})`,
              );
            }

            if (status.includes('BLOCKED')) {
              condition.push(
                sql.unsafe`'BLOCKED_USER' = ANY(${columns.userGroupNids})`,
              );
            }

            where.push(
              condition.length
                ? sql.unsafe`(${sql.join(condition, sql.fragment` OR `)})`
                : sql.fragment`true`,
            );
          }

          if (roleName) {
            where.push(sql.fragment`
              exists(
                          SELECT
                            *
                          FROM professional_role_user_account prua1
                          INNER JOIN professional_role pr1 ON
                            prua1.professional_role_id = pr1.id
                          WHERE
                            prua1.user_account_id = ${columns.id}
                            AND pr1.name ILIKE (${roleName} || '%')
                        )
            `);
          }

          if (googlePlaceId) {
            where.push(sql.fragment`
              ST_Intersects(
                          (
                            SELECT
                              ST_MakeEnvelope(
                                ST_X(gp1.viewport_southwest),
                                ST_Y(gp1.viewport_southwest),
                                ST_X(gp1.viewport_northeast),
                                ST_Y(gp1.viewport_northeast),
                                4326
                              )
                            FROM google_place gp1
                            WHERE
                              gp1.id = ${googlePlaceId}
                          ),
                          (
                            SELECT
                              ST_MakeEnvelope(
                                ST_X(gp1.viewport_southwest),
                                ST_Y(gp1.viewport_southwest),
                                ST_X(gp1.viewport_northeast),
                                ST_Y(gp1.viewport_northeast),
                                4326
                              )
                            FROM google_place gp1
                            WHERE
                              gp1.id = ${columns.googlePlaceId}
                          )
                        )
            `);
          }
        }

        return where.length
          ? sql.unsafe`${sql.join(where, sql.fragment` AND `)}`
          : sql.fragment`true`;
      },
    });
  });

I think that adding an option to ignore such errors would have the downside of making it easier to make mistakes in future...

I get that. I have strong opinions about codebases too, so not surprised to face resistance.

The sole purpose of this issue is to highlight the need of such option for real-world projects that go beyond simple queries.