Add dry run option

[stevehansen]

Summary

• add --dry-run option to list referenced views
• provide helper DatabaseView.GetReferencedViews
• document dry-run usage
• test GetReferencedViews

Testing

• dotnet test

[gemini-code-assist]

Hello @stevehansen, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello! Gemini here, providing a summary of this pull request. This PR introduces a new --dry-run option to the SqlInliner command-line tool. The primary goal is to allow users to see all the views that would be recursively referenced and potentially inlined for a given target view, without actually performing the inlining or outputting the final SQL. This is achieved by adding a new static helper method DatabaseView.GetReferencedViews which traverses the view dependencies. The PR also includes documentation updates for the new option and a unit test for the GetReferencedViews helper.

Highlights

• New Feature: Dry Run Option: Adds a --dry-run (-dr) command-line option to the SqlInliner tool. When this option is used, the tool will list all recursively referenced views for the target view and then exit, without generating the inlined SQL.
• Helper Method for View References: Introduces a new static method DatabaseView.GetReferencedViews that takes a DatabaseConnection and the SQL definition of a view, and returns a dictionary containing the target view and all views it recursively references.
• Documentation Update: The README.md file has been updated to include the new --dry-run option in the 'Additional options' section.
• Unit Testing: A new unit test GetReferencedViewsReturnsAllViews has been added to AdditionalTests.cs to verify the correctness of the new DatabaseView.GetReferencedViews method, ensuring it finds all views in a simple dependency chain.

Changelog

Click here to see the changelog

• README.md
  • Updated the description of 'Additional options' to reflect more than two options (line 60).
  • Added documentation for the new --dry-run (-dr) option (line 64).
• src/SqlInliner.Tests/AdditionalTests.cs
  • Added a new test method GetReferencedViewsReturnsAllViews to test the DatabaseView.GetReferencedViews method (lines 50-65).
• src/SqlInliner/DatabaseView.cs
  • Added using System; (line 3).
  • Implemented the static method GetReferencedViews which recursively finds and returns all views referenced by a given view SQL (lines 88-124).
• src/SqlInliner/Program.cs
  • Defined the new dryRunOption for the command line interface (line 20).
  • Added dryRunOption to the list of options for the root command (line 31).
  • Updated the rootCommand.SetHandler signature to use a context parameter instead of explicit parameters (line 37).
  • Retrieved the value of the new dryRun option from the parse context (line 45).
  • Added conditional logic to check if dryRun is true, call GetReferencedViews, print the view names, and return early (lines 69-75).
  • Updated the rootCommand.SetHandler call to remove the explicit parameter list (line 97).

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

Check the view tree,
Before the code runs free,
Dry run shows the way.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a valuable --dry-run option and the DatabaseView.GetReferencedViews helper method, which is a great addition for understanding view dependencies. The implementation is largely well-done, and the new functionality is clearly documented and tested.

I've reviewed the changes based on general C#/.NET coding conventions, focusing on correctness, efficiency, and maintainability. There's one area for potential improvement regarding the recursive nature of GetReferencedViews which I've detailed below. Overall, good work!

Summary of Findings

• Potential StackOverflowException in GetReferencedViews: The recursive approach in DatabaseView.GetReferencedViews (via the local AddView function) could lead to a StackOverflowException if view dependencies are very deep. An iterative approach using a stack or queue would be more robust. This was raised as a review comment.
• Minor: Missing newline at end of file (src/SqlInliner/DatabaseView.cs): A newline character is missing at the end of the DatabaseView.cs file (after line 125). This is a minor stylistic issue and was not raised as a formal review comment due to severity settings, but it's good practice to ensure files end with a newline.
• Observation: Error handling in GetReferencedViews for nested views: In DatabaseView.GetReferencedViews, when parsing nested views (line 110: var (referencedView, _) = FromSql(...)), parsing errors are currently ignored. While this might be acceptable for a utility that aims to list successfully parsed views, it means that if a dependent view is broken or unparsable, it (and its own dependencies) will be silently omitted from the results. This was not raised as a formal review comment due to severity settings, as the current behavior might be intended for a 'best-effort' listing.

Merge Readiness

The pull request introduces a useful --dry-run feature and is generally well-implemented. There is one medium-severity suggestion regarding the use of recursion in DatabaseView.GetReferencedViews that could lead to StackOverflowException in extreme cases of deeply nested views. I recommend addressing this by considering an iterative approach for improved robustness.

Once this point is considered, the PR should be in good shape for merging. As a reviewer, I am not authorized to approve the pull request; please ensure further review and approval as per your team's process.

[gemini-code-assist]

The current recursive implementation of AddView (and its usage) to discover all referenced views is clear for moderately nested views. However, for very deep view dependency chains, this recursive approach could potentially lead to a StackOverflowException.

Have you considered an iterative approach using a Stack<NamedTableReference> or Queue<NamedTableReference to traverse the view dependencies? This would make the method more robust against extremely deep recursion levels.

An iterative structure might look something like this:

// Initialize a stack with the direct references from the initial view
var viewsToProcess = new Stack<NamedTableReference>();
foreach (var initialReference in view.References.Views)
{
    viewsToProcess.Push(initialReference);
}

// Process the stack until empty
while (viewsToProcess.Count > 0)
{
    var currentViewReference = viewsToProcess.Pop();
    var viewName = currentViewReference.SchemaObject.GetName();

    // Skip if already processed
    if (knownViews.ContainsKey(viewName))
        continue;

    // Get and parse the referenced view definition
    var referencedDefinition = connection.GetViewDefinition(viewName);
    var (referencedView, _) = FromSql(connection, referencedDefinition); // Consider if errors here need specific handling
    
    if (referencedView == null)
        continue; // Or log/handle error: couldn't parse this referenced view

    knownViews[viewName] = referencedView;

    // Add its nested views to the stack for further processing
    foreach (var nestedReference in referencedView.References.Views)
    {
        viewsToProcess.Push(nestedReference);
    }
}

This would replace the local AddView function and the loop that initially calls it.

        // Iterative approach to discover views
        var viewsToProcess = new Stack<NamedTableReference>();
        foreach (var initialReference in view.References.Views)
        {
            viewsToProcess.Push(initialReference);
        }

        while (viewsToProcess.Count > 0)
        {
            var currentViewReference = viewsToProcess.Pop();
            var viewName = currentViewReference.SchemaObject.GetName();

            if (knownViews.ContainsKey(viewName))
                continue;

            var referencedDefinition = connection.GetViewDefinition(viewName);
            // Consider logging or collecting errors from FromSql if needed for diagnostics
            var (referencedView, /* parseErrors */ _) = FromSql(connection, referencedDefinition); 
            
            if (referencedView == null)
            {
                // Optionally, log that this view could not be parsed or found
                continue; 
            }

            knownViews[viewName] = referencedView;

            // Add newly discovered nested views to the stack
            foreach (var nestedReference in referencedView.References.Views)
            {
                viewsToProcess.Push(nestedReference);
            }
        }

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

The call to 'connection.GetViewDefinition(viewName)' can throw a NullReferenceException if the connection's Connection property is null (when using the parameterless constructor), or an exception if the view doesn't exist in the database. Consider adding error handling to gracefully handle missing or inaccessible view definitions instead of allowing the method to fail.

Suggested change

	var referencedDefinition = connection.GetViewDefinition(viewName);
	string referencedDefinition;
	try
	{
	referencedDefinition = connection.GetViewDefinition(viewName);
	}
	catch (Exception)
	{
	// Could not retrieve the view definition (view missing or connection issue), skip this view.
	return;
	}