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


##########
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:
   Hi @zeroshade ,
   Nice. That's something that I have not known about. Thanks for pointing it 
out. I have tried to create a different version with 
[NewGetTablesPaginator](https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/service/glue#NewGetTablesPaginator)
 and creating few extra test cases in glue catalog to verify its functionality 
:pray: 



-- 
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