tree: also remove existing hints from target during hint injection

During hint injection, also remove existing hints from the target
statement if they are not present in the donor statement. This makes it
easier to understand the rewrite performed by hint injection, as well as
makes it possible for hint injection to remove hints.

Informs: #153633

Release note: None

Author: michae2

pkg/sql/sem/tree/inject_hints.go

@@ -211,12 +211,13 @@ func (v *hintInjectionVisitor) VisitTablePre(expr TableExpr) (recurse bool, newE
 	donorExpr := v.hd.walk[v.walkIdx]
 	v.walkIdx += 1
 
-	// Copy hints from the corresponding donor TableExpr.
+	// Remove any existing hints from the original TableExpr, and copy hints from
+	// the corresponding donor TableExpr.
 	switch donor := donorExpr.(type) {
 	case *AliasedTableExpr:
 		switch orig := expr.(type) {
 		case *AliasedTableExpr:
-			if donor.IndexFlags != nil {
+			if !donor.IndexFlags.Equal(orig.IndexFlags) {
 				// Create a new node with any pre-existing inline hints replaced with
 				// hints from the donor.
 				newNode := *orig

pkg/sql/sem/tree/inject_hints_test.go

@@ -82,7 +82,7 @@ func TestInjectHints(t *testing.T) {
 			originalSQL:   "SELECT * FROM t@idx",
 			donorSQL:      "SELECT * FROM t@idx",
 			expectedSQL:   "SELECT * FROM t@idx",
-			expectChanged: true, // we still perform a rewrite, and still return true
+			expectChanged: false,
 		},
 		{
 			name:          "different constants should work",
@@ -153,11 +153,11 @@ func TestInjectHints(t *testing.T) {
 			expectChanged: true,
 		},
 		{
-			name:          "hint unchanged by donor",
+			name:          "hint overwritten by donor",
 			originalSQL:   "SELECT * FROM t@idx",
-			donorSQL:      "SELECT * FROM t",
-			expectedSQL:   "SELECT * FROM t@idx",
-			expectChanged: false,
+			donorSQL:      "SELECT * FROM t@{NO_FULL_SCAN}",
+			expectedSQL:   "SELECT * FROM t@{NO_FULL_SCAN}",
+			expectChanged: true,
 		},
 		{
 			name:          "subquery in join condition",
@@ -287,6 +287,13 @@ func TestInjectHints(t *testing.T) {
 			expectedSQL:   "SELECT * FROM a INNER JOIN b ON true",
 			expectChanged: true,
 		},
+		{
+			name:          "remove hints",
+			originalSQL:   "SELECT * FROM a@{NO_FULL_SCAN,IGNORE_FOREIGN_KEYS} INNER LOOKUP JOIN b@b_c_idx ON true",
+			donorSQL:      "SELECT * FROM a JOIN b ON true",
+			expectedSQL:   "SELECT * FROM a JOIN b ON true",
+			expectChanged: true,
+		},
 	}
 
 	st := cluster.MakeTestingClusterSettings()

pkg/sql/sem/tree/select.go

@@ -16,6 +16,7 @@ package tree
 
 import (
 	"fmt"
+	"slices"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
@@ -480,6 +481,28 @@ func (ih *IndexFlags) Check() error {
 	return nil
 }
 
+// Equal returns true if these IndexFlags equal the other IndexFlags.
+func (ih *IndexFlags) Equal(other *IndexFlags) bool {
+	if ih == nil || other == nil {
+		return ih == other
+	}
+	return ih.Index == other.Index &&
+		ih.IndexID == other.IndexID &&
+		ih.Direction == other.Direction &&
+		ih.NoIndexJoin == other.NoIndexJoin &&
+		ih.NoZigzagJoin == other.NoZigzagJoin &&
+		ih.NoFullScan == other.NoFullScan &&
+		ih.AvoidFullScan == other.AvoidFullScan &&
+		ih.IgnoreForeignKeys == other.IgnoreForeignKeys &&
+		ih.IgnoreUniqueWithoutIndexKeys == other.IgnoreUniqueWithoutIndexKeys &&
+		ih.ForceInvertedIndex == other.ForceInvertedIndex &&
+		ih.ForceZigzag == other.ForceZigzag &&
+		slices.Equal(ih.ZigzagIndexes, other.ZigzagIndexes) &&
+		slices.Equal(ih.ZigzagIndexIDs, other.ZigzagIndexIDs) &&
+		((ih.FamilyID == nil && other.FamilyID == nil) ||
+			(ih.FamilyID != nil && other.FamilyID != nil && *ih.FamilyID == *other.FamilyID))
+}
+
 var enableFamilyIDIndexHintForTests = false
 
 // TestingEnableFamilyIndexHint enables the use of Family index hint