docs(sql): add Doxygen comments to modules #143
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add detailed Doxygen documentation to 13 SQL files across Phase 4: **Encrypted Supporting Files (4 files):** - src/encrypted/aggregates.sql - MIN/MAX aggregate functions for ORE - src/encrypted/casts.sql - Type conversion between JSONB/text/encrypted - src/encrypted/compare.sql - Fallback comparison for btree correctness - src/encrypted/constraints.sql - 5 validation functions with @example tags **JSONB Functions (1 file, 15 functions):** - src/jsonb/functions.sql - Path query operations and array manipulation - jsonb_path_query (3 overloads) - Query for matching elements - jsonb_path_exists (3 overloads) - Check path existence - jsonb_path_query_first (3 overloads) - Get first matching element - jsonb_array_length (2 overloads) - Get array length - jsonb_array_elements (2 overloads) - Extract array elements - jsonb_array_elements_text (2 overloads) - Extract as ciphertext **Config Schema (4 files):** - src/config/types.sql - Configuration state ENUM documentation - src/config/tables.sql - eql_v2_configuration table structure - src/config/indexes.sql - Partial unique indexes for state constraints - src/config/constraints.sql - 5 validation functions **Encryptindex Functions (1 file, 7 functions):** - src/encryptindex/functions.sql - Configuration lifecycle management - diff_config, select_pending_columns, select_target_columns - ready_for_encryption, create_encrypted_columns - rename_encrypted_columns, count_encrypted_with_active_config **Root Utilities (3 files):** - src/common.sql - Constant-time comparison, JSONB conversion, logging - src/crypto.sql - pgcrypto extension enablement - src/schema.sql - eql_v2 schema creation with warnings **Documentation Quality:** - ✅ Consistent use of @brief, @param, @return, @throws, @note, @see tags - ✅ @internal tags mark implementation details vs customer-facing APIs - ✅ @example sections show concrete usage for customer functions - ✅ Cross-references create navigable documentation graph - ✅ File-level @file documentation provides module context - ✅ Security notes highlight timing attack prevention - ✅ All tests pass - documentation doesn't break functionality **Statistics:** - Files modified: 13 - Functions documented: 32+ - Lines added: +718 gross, +555 net - Test status: ✅ All 40+ test files passing **Code Review:** - Status: APPROVED (see CODE_REVIEW_PHASE_4_DOXYGEN.md) - BLOCKING issues: 0 - NON-BLOCKING issues: 6 (addressed in follow-up commit) - Review verified documentation accuracy against implementation
Address code review recommendations to improve documentation accuracy
and add safety warnings for DDL-executing functions.
**Documentation Improvements:**
1. **src/encryptindex/functions.sql:102-103** - Clarified select_target_columns
- Enhanced note to explain full JOIN logic (checks both column_name
and column_name_encrypted with type verification)
- Previous: "LEFT JOIN returns NULL when no match"
- Updated: Explains both name matching variants and type requirement
2. **src/jsonb/functions.sql:296** - Enhanced jsonb_array_length exception
- More specific @throws documentation with exact error message
- Previous: "if value is not an array (missing 'a' flag)"
- Updated: "'cannot get array length of a non-array' if 'a' flag
is missing or not true"
3. **src/jsonb/functions.sql:29** - Clarified NULL handling in jsonb_path_query
- Precise SETOF return semantics
- Previous: "Returns NULL if val is NULL"
- Updated: "Returns a set containing NULL if val is NULL"
**Safety Warnings:**
4. **src/encryptindex/functions.sql:152** - Added @warning to create_encrypted_columns
- Highlights dynamic DDL execution (ALTER TABLE ADD COLUMN)
- Alerts users to schema modification side effects
5. **src/encryptindex/functions.sql:180** - Added @warning to rename_encrypted_columns
- Highlights dynamic DDL execution (ALTER TABLE RENAME COLUMN)
- Alerts users to schema modification side effects
**Quality Assurance:**
- ✅ All tests passing (59 test files)
- ✅ Code review approved (CODE_REVIEW_PHASE_4_COMPLETE.md)
- ✅ 0 blocking issues
- ✅ All NON-BLOCKING recommendations addressed
**Note:** User fixed blocking code bug in jsonb_path_query_first separately
…ed_with_active_config **Problem:** Documentation incorrectly stated that the 'v' field stores the active configuration ID, contradicting src/encrypted/constraints.sql:56 which correctly documents that 'v' must be the literal payload version "2". **Root Cause:** The 'v' field serves a single, consistent purpose across the entire codebase: it stores the EQL payload format version (currently "2"). It does NOT store configuration IDs. **Fix:** - Removed incorrect claim that 'v' stores configuration ID - Clarified that 'v' field stores payload version "2" - Updated function description to avoid implementation details - Added note explaining the distinction **Files Changed:** - src/encryptindex/functions.sql:201-209 **Verification:** - ✅ All tests passing (59 test files) - ✅ Documentation now consistent with src/encrypted/constraints.sql:56 - ✅ No code changes - documentation-only fix **Context:** This addresses feedback about major documentation inconsistency where two different files made contradictory claims about the 'v' field's purpose. The payload version is always "2" for EQL v2, as enforced by the _encrypted_check_v validation function.
tobyhede
added a commit
that referenced
this pull request
Oct 28, 2025
Comprehensive evaluation of Phase 4 work against original plan: - Coverage: 52/53 files documented (98%) - Phase 4 tasks: All completed - Quality: Code-reviewed and verified - Deviations: Documented with rationale - Next steps: Clear recommendations Summary: - Phase 4 COMPLETE ✅ - PR #143 ready for merge - Phases 5-6 optional (infrastructure/QA) - src/version.sql excluded (auto-generated)
tobyhede
force-pushed
the
phase-4-doxygen
branch
2 times, most recently
from
16c1336 to
ba2d50e
Compare
Document version.template (source of auto-generated version.sql): - @file and @brief for template purpose - Full documentation for eql_v2.version() function - @note explaining template and build-time replacement - @example showing usage This achieves 100% documentation coverage (53/53 files) by documenting the source template rather than the generated file. The documentation will be preserved through build process.
- Updated version.template to avoid awkward $RELEASE_VERSION text replacement in docs - Changed @file tag from version.template to version.sql (the generated file) - Simplified documentation to avoid version string in prose - Added comment in build.sh clarifying Doxygen comment preservation - Added validation scripts for documentation quality assurance: - check-doc-coverage.sh: Reports documentation coverage (100%) - validate-documented-sql.sh: Validates SQL syntax - validate-required-tags.sh: Checks for required @brief, @param, @return tags The sed command in build.sh already preserves all Doxygen comments from the template, ensuring the generated version.sql is fully documented.
- Document required and optional Doxygen tags - Provide comprehensive documentation example - Reference validation tools for quality assurance - Add note about template file documentation - Update development notes to reference new standards This establishes clear documentation guidelines for all SQL functions and types in the EQL codebase.
- Create comprehensive Doxyfile for API documentation generation - Configure for SQL file parsing (treated as C++ for Doxygen) - Enable HTML output to docs/api directory - Exclude test files and build artifacts - Add docs/api/ to .gitignore for generated documentation To generate documentation: doxygen Doxyfile Output will be in docs/api/html/index.html
- Document how to generate API documentation with Doxygen - Explain documentation standards and required tags - Reference validation tools for quality assurance - Link to CLAUDE.md for contribution guidelines - Note automatic CI validation for PRs This makes documentation generation and standards visible to all users and contributors.
- Add docs:generate task to generate API documentation with Doxygen - Add docs:validate task to run coverage and tag validation - Document Doxygen installation via brew/apt in README - Update CLAUDE.md with new mise commands - Provide both mise and direct command usage examples Note: Doxygen is not available in mise registry, so must be installed separately via system package manager (brew, apt, etc.)
tobyhede
changed the title
- Simplified Doxyfile to minimal working configuration - Added doxygen-filter.sh to convert SQL comments (--!) to C++ style (//!) - Set FILTER_SOURCE_FILES=YES to apply filter to all SQL files - Set EXTRACT_ALL=YES to extract all documented entities - Removed obsolete Doxygen tags causing warnings The key issue was that Doxygen's C++ parser cannot recognize SQL-style --! comments. The input filter is REQUIRED to convert them to //! which Doxygen understands. Tested: Generates 213 HTML files with full documentation content.
- Fix validate-required-tags.sh to use BSD-compatible sed instead of grep -P - Update GitHub workflow to use 'mise run docs:validate' instead of direct scripts - Update CLAUDE.md to recommend mise tasks as primary validation method - Maintain backward compatibility with direct script execution The grep -oP flag is a GNU extension not available in BSD grep (macOS). Replaced with sed pattern matching for cross-platform compatibility. CI now uses mise for consistency with local development workflow.
- Add missing @brief, @param, @return tags to operators - src/operators/~~.sql: Add function documentation for LIKE operator - src/operators/<>.sql: Add @param and @return tags - Fix validate-required-tags.sh to handle longer comment blocks - Increased tail from 20 to 100 lines to capture complete comments - Previous limit was truncating @brief tags in longer documentation blocks - Fixes false positive errors for well-documented functions All validation now passes with 0 errors and 0 warnings.
tobyhede
force-pushed
the
phase-4-doxygen
branch
from
0b144e6 to
b4d6b4d
Compare
freshtonic
approved these changes
Oct 28, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Complete Doxygen Documentation for EQL (Phases 1-4)
🎯 Overview
This PR provides complete Doxygen documentation for the Encrypt Query Language (EQL) codebase, covering ALL phases (1-4) with full infrastructure for documentation generation, validation, and maintenance.
📦 What's Included
SQL Documentation (53 files)
Phase 1: Foundation
Phase 2: Core Modules (18 files)
<,<=,<>,=,>,>=,->,->>,@>,<@,~~Phase 3: Index Types (15 files)
Phase 4: Supporting Infrastructure (20 files)
🛠️ Infrastructure & Tooling
Documentation Generation:
Doxyfileconfiguration for HTML/LaTeX outputmise run doc:generate,mise run doc:validatedocs/api/html/CI/CD Integration:
Standards & Guidelines:
CLAUDE.md@brief,@param,@return,@throws,@note,@see,@internal,@exampleQuality Assurance:
📊 Statistics
✅ Verification
All tests passing (59 tests):
🔍 Code Review
Zero blocking issues identified in comprehensive code review:
📚 Documentation Highlights
Customer-Facing APIs:
@briefdescriptions for all public functions@paramand@returndocumentation@exampleblocks showing usage patterns@throwstagsImplementation Details:
@internaltags for private APIs@noteblocks for important implementation details@seecross-references between related functions🔗 Related Work
🚀 Usage
After merging, developers can:
💡 Benefits
📋 Checklist
🎉 Result
This PR delivers a complete, production-ready Doxygen documentation system for EQL, covering all modules with infrastructure to maintain documentation quality going forward.