zeroshade commented on code in PR #306:
URL: https://github.com/apache/iceberg-go/pull/306#discussion_r1962080682


##########
catalog/glue/glue.go:
##########
@@ -170,28 +170,32 @@ func (c *Catalog) ListTables(ctx context.Context, 
namespace table.Identifier) it
                                        return
                                }
                        }
-                       if nextPageToken == nil {
+                       if nextPageToken == "" {
                                return
                        }
                }
        }
 }
 
-func (c *Catalog) ListTablesPage(ctx context.Context, namespace 
table.Identifier) ([]table.Identifier, *string, error) {
+func (c *Catalog) listTablesPage(ctx context.Context, namespace 
table.Identifier) ([]table.Identifier, string, error) {
        database, err := identifierToGlueDatabase(namespace)
        if err != nil {
-               return nil, nil, err
+               return nil, "", err
        }
        params := &glue.GetTablesInput{CatalogId: c.catalogId, DatabaseName: 
aws.String(database)}
        tblsRes, err := c.glueSvc.GetTables(ctx, params)
        if err != nil {
-               return nil, nil, fmt.Errorf("failed to list tables in namespace 
%s: %w", database, err)
+               return nil, "", fmt.Errorf("failed to list tables in namespace 
%s: %w", database, err)
        }
        var icebergTables []table.Identifier
        icebergTables = append(icebergTables,
                filterTableListByType(database, tblsRes.TableList, 
glueTypeIceberg)...)
 
-       return icebergTables, tblsRes.NextToken, nil
+       var nextToken string
+       if tblsRes.NextToken != nil {
+               nextToken = *tblsRes.NextToken
+       }
+       return icebergTables, nextToken, nil
 }

Review Comment:
   looking at the docs for 
[`GetTablesInput`](https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/glue#GetTablesInput)
 it looks like the `nextToken` should be being passed back to `listTablesPage` 
and put into the `GetTablesInput`. 
   
   Personally, it might make more sense to just use 
[NewGetTablesPaginator](https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/glue#NewGetTablesPaginator)
 instead, rather than implementing the pagination ourselves :smile:



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to