Skip to content

Fix index for OWASP grouping

Bala Kumar requested to merge 473748-fix-index-for-owasp-grouping into master

What does this MR do and why?

Ensures that unnest optimizations are applied using the query rewritter for the group level owasp_top_10 filtering.

Without this index the query rewriter abstraction does not apply unnest optimization. And it was also noted that resolved_on_default_branch should be before severity for better performance here.

Old index will be removed in a follow up issue - #483367 (closed)

Database review

bundle exec rake db:migrate:up:main VERSION=20240828173211
main: == [advisory_lock_connection] object_id: 128160, pg_backend_pid: 36398
main: == 20240828173211 AddRevisedIdxOwaspTop10ForGroupLevelReports: migrating ======
main: -- transaction_open?(nil)
main:    -> 0.0000s
main: -- view_exists?(:postgres_partitions)
main:    -> 0.0278s
main: -- index_exists?(:vulnerability_reads, [:owasp_top_10, :state, :report_type, :resolved_on_default_branch, :severity, :traversal_ids, :vulnerability_id], {:where=>"archived = false", :name=>"revised_idx_for_owasp_top_10_group_level_reports", :algorithm=>:concurrently})
main:    -> 0.0077s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0005s
main: -- add_index(:vulnerability_reads, [:owasp_top_10, :state, :report_type, :resolved_on_default_branch, :severity, :traversal_ids, :vulnerability_id], {:where=>"archived = false", :name=>"revised_idx_for_owasp_top_10_group_level_reports", :algorithm=>:concurrently})
main:    -> 0.0076s
main: -- execute("RESET statement_timeout")
main:    -> 0.0006s
main: == 20240828173211 AddRevisedIdxOwaspTop10ForGroupLevelReports: migrated (0.0619s)

main: == [advisory_lock_connection] object_id: 128160, pg_backend_pid: 36398
rake db:migrate:down:main VERSION=20240828173211
main: == [advisory_lock_connection] object_id: 128160, pg_backend_pid: 37092
main: == 20240828173211 AddRevisedIdxOwaspTop10ForGroupLevelReports: reverting ======
main: -- transaction_open?(nil)
main:    -> 0.0000s
main: -- view_exists?(:postgres_partitions)
main:    -> 0.0256s
main: -- indexes(:vulnerability_reads)
main:    -> 0.0101s
main: -- execute("SET statement_timeout TO 0")
main:    -> 0.0003s
main: -- remove_index(:vulnerability_reads, {:algorithm=>:concurrently, :name=>"revised_idx_for_owasp_top_10_group_level_reports"})
main:    -> 0.0025s
main: -- execute("RESET statement_timeout")
main:    -> 0.0003s
main: == 20240828173211 AddRevisedIdxOwaspTop10ForGroupLevelReports: reverted (0.0524s)

main: == [advisory_lock_connection] object_id: 128160, pg_backend_pid: 37092
Current OWASP filter query without unnest in filters using the earlier index:

https://postgres.ai/console/gitlab/gitlab-production-main/sessions/31286/commands/97122

Current OWASP filter query without unnest in filters using this new index:

https://console.postgres.ai/gitlab/gitlab-production-main/sessions/31286/commands/97131

Rewritten query with unnest in filters using this new index:

https://console.postgres.ai/gitlab/gitlab-production-main/sessions/31286/commands/97130

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

  1. Without the index the below relation in rails console with the rewriter abstraction would return false and with the index it will return true and calling rewriter.rewrite will construct the query with the unnest in filters.
  relation = Vulnerabilities::Read
  .where(
    {
      "archived" => false,
      "report_type" => [
        "api_fuzzing",
        "container_scanning",
        "coverage_fuzzing",
        "dast",
        "dependency_scanning",
        "sast",
        "secret_detection",
        "generic"
      ],
      "severity" => [1, 2, 4, 5, 6, 7],
      "state" => ["detected", "confirmed"],
      "resolved_on_default_branch" => false,
      "owasp_top_10" => "A10:2017-Insufficient Logging & Monitoring"
    }
  )
  .where("traversal_ids >= ? AND traversal_ids < ?", '{110}', '{111}')
  .order(severity: :desc, traversal_ids: :desc, vulnerability_id: :desc)
  .limit(101)

  rewriter = UnnestedInFilters::Rewriter.new(relation)

  rewriter.rewrite?

Related to #473748 (closed)

Edited by Bala Kumar

Merge request reports

Loading