Skip to content

Commit

Permalink
feat: create issues of todo comments
Browse files Browse the repository at this point in the history
  • Loading branch information
jeroenrinzema committed Oct 25, 2022
1 parent 4501319 commit 3dbc832
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 26 deletions.
20 changes: 20 additions & 0 deletions .github/workflows/todo.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
name: TODOs

on:
push:
branches:
- main

jobs:
scan:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Run tdg-github-action
uses: ribtoks/tdg-github-action@master
with:
TOKEN: ${{ secrets.GITHUB_TOKEN }}
REPO: ${{ github.repository }}
SHA: ${{ github.sha }}
REF: ${{ github.ref }}
LABEL: "todo"
2 changes: 1 addition & 1 deletion auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestClearTextPassword(t *testing.T) {
input := bytes.NewBuffer([]byte{})
incoming := buffer.NewWriter(input)

// NOTE(Jeroen): we could reuse the server buffered writer to write client messages
// NOTE: we could reuse the server buffered writer to write client messages
incoming.Start(types.ServerMessage(types.ClientPassword))
incoming.AddString(expected)
incoming.AddNullTerminate()
Expand Down
48 changes: 33 additions & 15 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ func NewErrUnkownStatement(name string) error {
func (srv *Server) consumeCommands(ctx context.Context, conn net.Conn, reader *buffer.Reader, writer *buffer.Writer) (err error) {
srv.logger.Debug("ready for query... starting to consume commands")

// TODO(Jeroen): include a identification value inside the context that
// TODO: Include a value to identify unique connections
//
// include a identification value inside the context that
// could be used to identify connections at a later stage.

for {
Expand All @@ -52,7 +54,7 @@ func (srv *Server) consumeCommands(ctx context.Context, conn net.Conn, reader *b
return nil
}

// NOTE(Jeroen): we could recover from this scenario
// NOTE: we could recover from this scenario
if errors.Is(err, buffer.ErrMessageSizeExceeded) {
err = srv.handleMessageSizeExceeded(reader, writer, err)
if err != nil {
Expand Down Expand Up @@ -112,7 +114,7 @@ func (srv *Server) handleCommand(ctx context.Context, conn net.Conn, t types.Cli

switch t {
case types.ClientSync:
// TODO(Jeroen): include the ability to catch sync messages in order to
// TODO: Include the ability to catch sync messages in order to
// close the current transaction.
//
// At completion of each series of extended-query messages, the frontend
Expand All @@ -136,7 +138,7 @@ func (srv *Server) handleCommand(ctx context.Context, conn net.Conn, t types.Cli
case types.ClientParse:
return srv.handleParse(ctx, reader, writer)
case types.ClientDescribe:
// TODO(Jeroen): the server should return the column types that will be
// TODO: Server should return the column types that will be
// returned for the given portal or statement.
//
// The Describe message (portal variant) specifies the name of an
Expand All @@ -161,8 +163,10 @@ func (srv *Server) handleCommand(ctx context.Context, conn net.Conn, t types.Cli
case types.ClientBind:
return srv.handleBind(ctx, reader, writer)
case types.ClientFlush:
// TODO(Jeroen): flush all remaining rows inside connection buffer if
// any are remaining. The Flush message does not cause any specific
// TODO: Flush all remaining rows inside connection buffer if
// any are remaining.
//
// The Flush message does not cause any specific
// output to be generated, but forces the backend to deliver any data
// pending in its output buffers. A Flush must be sent after any
// extended-query command except Sync, if the frontend wishes to examine
Expand Down Expand Up @@ -247,7 +251,7 @@ func (srv *Server) handleParse(ctx context.Context, reader *buffer.Reader, write
return err
}

// NOTE(Jeroen): the number of parameter data types specified (can be
// NOTE: the number of parameter data types specified (can be
// zero). Note that this is not an indication of the number of parameters
// that might appear in the query string, only the number that the frontend
// wants to prespecify types for.
Expand All @@ -257,9 +261,11 @@ func (srv *Server) handleParse(ctx context.Context, reader *buffer.Reader, write
}

for i := uint16(0); i < parameters; i++ {
// TODO(Jeroen): reader.GetUint32()
// TODO: Specifies the object ID of the parameter data type
//
// Specifies the object ID of the parameter data type. Placing a zero here
// is equivalent to leaving the type unspecified.
// `reader.GetUint32()`
}

statement, descriptions, err := srv.Parse(ctx, query)
Expand Down Expand Up @@ -328,7 +334,7 @@ func (srv *Server) handleBind(ctx context.Context, reader *buffer.Reader, writer
// reader. The parameters are parsed and returned.
// https://www.postgresql.org/docs/14/protocol-message-formats.html
func (srv *Server) readParameters(ctx context.Context, reader *buffer.Reader) ([]string, error) {
// NOTE(Jeroen): read the total amount of parameter format codes that will
// NOTE: read the total amount of parameter format codes that will
// be send by the client.
length, err := reader.GetUint16()
if err != nil {
Expand All @@ -343,16 +349,20 @@ func (srv *Server) readParameters(ctx context.Context, reader *buffer.Reader) ([
return nil, err
}

// NOTE(Jeroen): the parameter format codes. Each must presently be zero (text) or one (binary).
// NOTE: the parameter format codes. Each must presently be zero (text) or one (binary).
// https://www.postgresql.org/docs/14/protocol-message-formats.html
if format != 0 {
return nil, errors.New("unsupported binary parameter format, only text formatted parameter types are currently supported")
}

// TODO(Jeroen): handle multiple parameter format codes.
// TODO: Handle multiple parameter format codes.
//
// We are currently only supporting string parameters. We have to
// include support for binary parameters in the future.
// https://www.postgresql.org/docs/14/protocol-message-formats.html
}

// NOTE(Jeroen): read the total amount of parameter values that will be send
// NOTE: read the total amount of parameter values that will be send
// by the client.
length, err = reader.GetUint16()
if err != nil {
Expand All @@ -377,7 +387,7 @@ func (srv *Server) readParameters(ctx context.Context, reader *buffer.Reader) ([
parameters[i] = string(value)
}

// NOTE(Jeroen): read the total amount of result-column format that will be
// NOTE: Read the total amount of result-column format that will be
// send by the client.
length, err = reader.GetUint16()
if err != nil {
Expand All @@ -387,7 +397,13 @@ func (srv *Server) readParameters(ctx context.Context, reader *buffer.Reader) ([
srv.logger.Debug("reading result-column format codes", zap.Uint16("length", length))

for i := uint16(0); i < length; i++ {
// TODO(Jeroen): handle incoming result-column format codes
// TODO: Handle incoming result-column format codes
//
// Incoming format codes are currently ignored and should be handled in
// the future. The result-column format codes. Each must presently be
// zero (text) or one (binary). These format codes should be returned
// and handled by the parent function to return the proper column formats.
// https://www.postgresql.org/docs/current/protocol-message-formats.html
_, err := reader.GetUint16()
if err != nil {
return nil, err
Expand All @@ -407,7 +423,9 @@ func (srv *Server) handleExecute(ctx context.Context, reader *buffer.Reader, wri
return err
}

// TODO(Jeroen): Maximum number of limit to return, if portal contains a
// TODO: Limit the maximum number of records to be returned.
//
// Maximum number of limit to return, if portal contains a
// query that returns limit (ignored otherwise). Zero denotes “no limit”.
limit, err := reader.GetUint32()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestMessageSizeExceeded(t *testing.T) {
client.Authenticate(t)
client.ReadyForQuery(t)

// NOTE(Jeroen): attempt to send a message twice the max buffer size
// NOTE: attempt to send a message twice the max buffer size
size := uint32(buffer.DefaultBufferSize * 2)
t.Logf("writing message of size: %d", size)

Expand Down
10 changes: 8 additions & 2 deletions handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ func (srv *Server) Handshake(conn net.Conn) (_ net.Conn, version types.Version,
return conn, version, reader, nil
}

// TODO(Jeroen): support GSS encryption
// TODO: support GSS encryption
//
// `psql-wire` currently does not support GSS encrypted connections. The GSS
// authentication API is supported inside the PostgreSQL wire protocol and
// API's should be made available to support these type of connections.
// https://www.postgresql.org/docs/current/gssapi-auth.html
// https://www.postgresql.org/docs/current/protocol-flow.html#id-1.10.6.7.13

conn, reader, version, err = srv.potentialConnUpgrade(conn, reader, version)
if err != nil {
Expand Down Expand Up @@ -152,7 +158,7 @@ func (srv *Server) potentialConnUpgrade(conn net.Conn, reader *buffer.Reader, ve
ClientCAs: srv.ClientCAs,
}

// NOTE(Jeroen): initialize the TLS connection and construct a new buffered
// NOTE: initialize the TLS connection and construct a new buffered
// reader for the constructed TLS connection.
conn = tls.Server(conn, &tlsConfig)
reader = buffer.NewReader(conn, srv.BufferedMsgSize)
Expand Down
6 changes: 3 additions & 3 deletions internal/mock/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (client *Client) Handshake(t *testing.T) {
version := make([]byte, 4)
binary.BigEndian.PutUint32(version, uint32(types.Version30))

// NOTE(Jeroen): the parameters consist out of keys and values. Each key and
// NOTE: the parameters consist out of keys and values. Each key and
// value is terminated using a nul byte and the end of all parameters is
// identified using a empty key value.
nul := byte(0)
Expand All @@ -42,7 +42,7 @@ func (client *Client) Handshake(t *testing.T) {
end := append([]byte(""), nul)
parameters := append(append(key, value...), end...)

// NOTE(Jeroen): we have to define the total message length inside the
// NOTE: we have to define the total message length inside the
// header by prefixing a unsigned 32 big-endian int.
header := make([]byte, 4)
binary.BigEndian.PutUint32(header, uint32(len(version)+len(parameters)+len(header)))
Expand Down Expand Up @@ -74,7 +74,7 @@ func (client *Client) Authenticate(t *testing.T) {
t.Fatal(err)
}

// NOTE(Jeroen): a status of 0 indicates that the connection has been authenticated
// NOTE: a status of 0 indicates that the connection has been authenticated
if status != 0 {
t.Fatalf("unexpected auth status: %d, expected auth ok", status)
}
Expand Down
2 changes: 1 addition & 1 deletion options.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func SimpleQuery(fn SimpleQueryFn) OptionFn {
return fn(ctx, query, writer, parameters)
}

// NOTE(Jeroen): we have to lookup all unique positional parameters
// NOTE: we have to lookup all unique positional parameters
// within the given query. We return a zero parameter oid for each
// positional parameter indicating that the given parameters could
// contain any type. We could safely ignore the err check while
Expand Down
17 changes: 15 additions & 2 deletions row.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,20 @@ func (column Column) Define(ctx context.Context, writer *buffer.Writer) {
writer.AddInt16(column.AttrNo)
writer.AddInt32(int32(column.Oid))
writer.AddInt16(column.Width)
writer.AddInt32(-1) // TODO(Jeroen): type modifiers have not yet been fully implemented. Setting -1 to indicate a undefined value
// TODO: Support type for type modifiers
//
// Some types could be overridden using the type modifier field within a RowDescription.
// Type modifier (see pg_attribute.atttypmod). The meaning of the
// modifier is type-specific.
// Atttypmod records type-specific data supplied at table creation time (for
// example, the maximum length of a varchar column). It is passed to
// type-specific input functions and length coercion functions. The value
// will generally be -1 for types that do not need atttypmod.
//
// https://www.postgresql.org/docs/current/protocol-message-formats.html
// https://www.postgresql.org/docs/current/catalog-pg-attribute.html

writer.AddInt32(-1)
writer.AddInt16(int16(column.Format))
}

Expand Down Expand Up @@ -103,7 +116,7 @@ func (column Column) Write(ctx context.Context, writer *buffer.Writer, src any)
return err
}

// NOTE(Jeroen): The length of the column value, in bytes (this count does
// NOTE: The length of the column value, in bytes (this count does
// not include itself). Can be zero. As a special case, -1 indicates a NULL
// column value. No value bytes follow in the NULL case.
length := int32(len(bb))
Expand Down
2 changes: 1 addition & 1 deletion wire.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (srv *Server) Serve(listener net.Listener) error {

srv.wg.Add(1)

// NOTE(Jeroen): handle graceful shutdowns
// NOTE: handle graceful shutdowns
go func() {
defer srv.wg.Done()
<-srv.closer
Expand Down

0 comments on commit 3dbc832

Please sign in to comment.