From a90f64a1a56a0a7e2e7f09a0eada8402ea1e2579 Mon Sep 17 00:00:00 2001
From: Andy Arthur <andy@dolthub.com>
Date: Thu, 20 Oct 2022 12:17:01 -0700
Subject: [PATCH] Added doc comments to schema.Schema, integration test

---
 .../doltcore/schema/integration_test.go       | 46 +++++++++++++++++++
 go/libraries/doltcore/schema/schema.go        | 29 ++++++++++--
 2 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/go/libraries/doltcore/schema/integration_test.go b/go/libraries/doltcore/schema/integration_test.go
index ddfcf7a7b6..5d57434737 100644
--- a/go/libraries/doltcore/schema/integration_test.go
+++ b/go/libraries/doltcore/schema/integration_test.go
@@ -64,6 +64,52 @@ func TestSqlIntegration(t *testing.T) {
 	}
 }
 
+func TestSchemaOrdering(t *testing.T) {
+	tests := []struct {
+		name      string
+		query     string
+		pkCols    []string
+		otherCols []string
+		allCols   []string
+		ordinals  []int
+	}{
+		{
+			name:      "primary key",
+			query:     "CREATE TABLE t (a int, b int, pk2 int, c int, pk1 int, PRIMARY KEY (pk1, pk2));",
+			pkCols:    []string{"pk1", "pk2"},
+			otherCols: []string{"a", "b", "c"},
+			allCols:   []string{"a", "b", "pk2", "c", "pk1"},
+			ordinals:  []int{4, 2},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ctx := context.Background()
+			root := runTestSql(t, ctx, []string{test.query})
+
+			tbl, ok, err := root.GetTable(ctx, "t")
+			require.NoError(t, err)
+			require.True(t, ok)
+			sch, err := tbl.GetSchema(ctx)
+			require.NoError(t, err)
+
+			for i, col := range sch.GetPKCols().GetColumns() {
+				assert.Equal(t, test.pkCols[i], col.Name)
+			}
+			for i, col := range sch.GetNonPKCols().GetColumns() {
+				assert.Equal(t, test.otherCols[i], col.Name)
+			}
+			for i, col := range sch.GetAllCols().GetColumns() {
+				assert.Equal(t, test.allCols[i], col.Name)
+			}
+			for i, ord := range sch.GetPkOrdinals() {
+				assert.Equal(t, test.ordinals[i], ord)
+			}
+		})
+	}
+}
+
 func TestGetKeyTags(t *testing.T) {
 	const tblName = "test"
 	tests := []struct {
diff --git a/go/libraries/doltcore/schema/schema.go b/go/libraries/doltcore/schema/schema.go
index 01afd3316c..34ac18f720 100644
--- a/go/libraries/doltcore/schema/schema.go
+++ b/go/libraries/doltcore/schema/schema.go
@@ -26,16 +26,34 @@ import (
 	"github.com/dolthub/dolt/go/store/val"
 )
 
-// Schema is an interface for retrieving the columns that make up a schema
+// Schema defines the schema of a table and describes both its SQL schema and storage layout.
+//
+// For example, a SQL table defined as:
+//
+//	`CREATE TABLE t (a int, b int, pk2 int, c int, pk1 int, PRIMARY KEY (pk1, pk2));`
+//
+// Has a corresponding Schema of:
+//
+//	Schema {
+//		PkCols:     [pk1, pk2],
+//		NonPkCols:  [a, b, c],
+//		AllCols:    [a, b, pk2, c, pk1],
+//		PkOrdinals: [4, 2],
+//	}
 type Schema interface {
-	// GetPKCols gets the collection of columns which make the primary key. They
-	// are always returned in ordinal order.
+	// GetPKCols gets the collection of columns which make the primary key.
+	// Columns in this collection are ordered by storage order, which is
+	// defined in the 'PRIMARY KEY(...)' clause of a CREATE TABLE statement.
 	GetPKCols() *ColCollection
 
 	// GetNonPKCols gets the collection of columns which are not part of the primary key.
+	// Columns in this collection are ordered by schema order (display order), which is
+	// defined by the order of first occurrence in a CREATE TABLE statement.
 	GetNonPKCols() *ColCollection
 
 	// GetAllCols gets the collection of all columns (pk and non-pk)
+	// Columns in this collection are ordered by schema order (display order), which is
+	// defined by the order of first occurrence in a CREATE TABLE statement.
 	GetAllCols() *ColCollection
 
 	// Indexes returns a collection of all indexes on the table that this schema belongs to.
@@ -44,10 +62,11 @@ type Schema interface {
 	// Checks returns a collection of all check constraints on the table that this schema belongs to.
 	Checks() CheckCollection
 
-	// GetPkOrdinals returns a slice of the primary key ordering indexes relative to the schema column ordering
+	// GetPkOrdinals returns a slice of schema order positions for the primary key columns. These ith
+	// value of this slice contains schema position for the ith column in the PK ColCollection.
 	GetPkOrdinals() []int
 
-	// SetPkOrdinals specifies a primary key column ordering
+	// SetPkOrdinals specifies a primary key column ordering. See GetPkOrdinals.
 	SetPkOrdinals([]int) error
 
 	// AddColumn adds a column to this schema in the order given and returns the resulting Schema.